Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce PHPMailer library and Fix SMTP Mail send #658

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

Introduce PHPMailer library and Fix SMTP Mail send #658

wants to merge 35 commits into from

Conversation

andygrunwald
Copy link
Collaborator

Context

This Pull Request is the new version of #524.

While I wanted to make #524 mergable, I pushed to many changes to the PR. Hence, I created this new, clean one.

The kudos goes to @mrflobow.

Description from #524

This commit fixes #490.
For accomplishing the goal I added the libary phpmailer (as also suggested).
No big issues with adding it to composer.
I believe with using the library lansuite mail module gets future proof and can be used in wider server configuration.

Tested following combinations:

No Auth against local docker smtp container ->Success
With Auth against MailTrap - >Success
With TLS against Outlook. - >Success
For public SMTP you can't send as different user so I added an optional configuration to set the sender instead of the user itself. Example could be used with noreply@something.com .
If the configuration is left blank it will use the user E-Mail instead.

Summary:

Refactored Mail Module
Used widely use PHPMailer for sending mails instead of reimplenting from scratch.
Added new settings to make SMTP configurable as needed.
PHP Composer lock has been updated.

Tickets

Fixes #490
Related #524

@mrflobow @andygrunwald
* Refactored Mail Module
5b5b480 
* Used widely use PHPMailer for sending mails instead of reimplenting from scratch.
* Added new settings
** Set Port, Sender (opt) , TLS Yes/No
* PHP Composer lock has been updated.
@mrflobow @andygrunwald
Removed unused code
e5aff27
@mrflobow @andygrunwald
* Sonar Fixes
a48973a 
* Improved Mail Pattern
* Fixed wrong Encryption Pattern , explicit use TLS
@mrflobow @andygrunwald
* Sonar Fixes
6b8c919
@mrflobow @andygrunwald
* Sonar Fixes
165033e
@mrflobow @andygrunwald
- Sonar fixes
b95f769 
- Reduced complexity
- Added validator
@mrflobow @andygrunwald
phpcs fix
3f7c3f4
@andygrunwald andygrunwald requested a review from M4LuZ June 28, 2023 19:57
@andygrunwald andygrunwald mentioned this pull request Jun 28, 2023
Closed
@andygrunwald
Copy link
Collaborator Author

I made a few additional fixes. Mostly PHP typing to make it consistent.

Next step:

  • Setting up a proper dev environment for mail testing
  • Testing the changes

@M4LuZ M4LuZ mentioned this pull request Jun 29, 2023
Open
@M4LuZ
Copy link
Collaborator

M4LuZ commented Jun 29, 2023 •
edited

Test cases I see:

  • update case (switch from current codebase and config to this)
  • unauthenticated localhost sending
  • remote MTA with credentials
  • TLS handling
  • Error handling (does this queue or throw away unaccepted messages btw?)

Additional actions:

@andygrunwald
Copy link
Collaborator Author

andygrunwald commented Jun 29, 2023 •
edited

I planned to create a test setup with https://github.com/axllent/mailpit + document it on how you can run the tests locally.

@sonarcloud
Copy link

sonarcloud bot commented Aug 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Sep 14, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 26, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@M4LuZ M4LuZ M4LuZ left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

E-Mail communication does not support STARTTLS
3 participants
@andygrunwald @M4LuZ @mrflobow