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/08/24 20:16:19 UTC
[GitHub] [fineract] francisguchie opened a new pull request #1837: FINERACT-1389 add starttls and factory
francisguchie opened a new pull request #1837:
URL: https://github.com/apache/fineract/pull/1837
## Description
Please read https://issues.apache.org/jira/browse/FINERACT-1389
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] eroemma commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
eroemma commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-905369892
Your contributions have impacted us positively in financial inclusion with Mifos x Thank you @francisguchie
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola merged pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #1837:
URL: https://github.com/apache/fineract/pull/1837
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] BLasan commented on a change in pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#discussion_r695643979
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
##########
@@ -73,15 +73,15 @@ public void sendDefinedEmail(EmailDetail emailDetails) {
props.put("mail.smtp.auth", "true");
props.put("mail.debug", "true");
- try {
- if (smtpCredentialsData.isUseTLS()) {
- // Needs to disable startTLS if the port is 465 in order to send the email successfully when using the
- // smtp.gmail.com as the host
- if (smtpCredentialsData.getPort().equals("465")) {
- props.put("mail.smtp.starttls.enable", "false");
- }
- }
+ // these are the added lines
+ props.put("mail.smtp.starttls.enable", "true");
+ // props.put("mail.smtp.ssl.enable", "true");
Review comment:
What if the SMTP server uses SSL? https://stackoverflow.com/questions/411331/using-javamail-with-tls
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-907817269
@BLasan,
Is there anything else left
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] BLasan commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-919203968
> @BLasan,
> Is there anything else left
Nothing from my side. LGTM
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-905034324
@BLasan please look at this
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-919588123
@BLasan , I am already using this in live production waiting for the merger
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#issuecomment-905456424
@BLasan please note that this has been tested on smtp.gmail on port 587 i have also read around and found that smtp.mail.yahoo uses 465 and a few others. Most mail providers are always updating their connection properties and i think we should always visit this email code as also the java jars too are being updated most of the time to address changes from the email service providers. The only users who will enjoy this module with little or no challenge are those using their private email servers they have all options to use like enabling TLS or not , changing the ports etc.
as for common email service providers (gmail, yahoo) , port 25 is a no go area we will be forced to say in the 465 and 587 lane mainly
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on a change in pull request #1837: FINERACT-1389 add starttls and factory
Posted by GitBox <gi...@apache.org>.
francisguchie commented on a change in pull request #1837:
URL: https://github.com/apache/fineract/pull/1837#discussion_r695695411
##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
##########
@@ -73,15 +73,15 @@ public void sendDefinedEmail(EmailDetail emailDetails) {
props.put("mail.smtp.auth", "true");
props.put("mail.debug", "true");
- try {
- if (smtpCredentialsData.isUseTLS()) {
- // Needs to disable startTLS if the port is 465 in order to send the email successfully when using the
- // smtp.gmail.com as the host
- if (smtpCredentialsData.getPort().equals("465")) {
- props.put("mail.smtp.starttls.enable", "false");
- }
- }
+ // these are the added lines
+ props.put("mail.smtp.starttls.enable", "true");
+ // props.put("mail.smtp.ssl.enable", "true");
Review comment:
@BLasan - I read around and discovered that by triggering starttls.enable = true, intuitively if a server uses SSL this will still deliver the email. For example I have tested this code on a server that has Domain Validation Certificate verified by lets Encrypt (https://letsencrypt.org/). and it still works.
Our biggest challenge is the email is being sent as plaintext and if we enable props.put("mail.smtp.ssl.enable", "true"); then the we will get this error - javax.net.ssl.SSLException: Unsupported or unrecognized SSL message and i went ahead to look into what others say like here https://stackoverflow.com/questions/61747336/javax-net-ssl-sslexception-unsupported-or-unrecognized-ssl-message
In conclusion when i disabled this property all worked fine for create users , self service users etc it all works fine for me atleast
--
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.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org