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.

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

Add default SMTP authentication type to NewClient #335

Conversation

wneessen
Copy link
Owner

This PR fixes a regression introduced in v0.5.0. SMTPAuthNoAuth is now changed from an empty string to "NOAUTH". This value is set as default for the Client in NewClient. This way we always have a fixed assignment and an empty string would not skip authentication. The auth() method has been updated to either assign the smtp.Auth function if SMTPAuthType is not set to "NOAUTH" or skip the part there is already an auth function set (this would only happen when SetSMTPAuthCustom or "WithSMTPAuthCustom were used.

If SMTPAuthType is set to an empty string, the authentication assignment would fail as it is a not supported mechanism, therefore making sure that the client wouldn't accidentaly skip the authentication at all.

A test has been added to test different auth switching scenarios (using supported, unsupported and custom auth functions). The documentation for auth() has been updated accordingly to make it more clear, what's happening.

This should fix the regression described in #332 and #328 but should also address the "empty string" issue, we had before.

This commit fixes a regression introduced in v0.5.0. We now set the default SMTPAuthType to NOAUTH in NewClient. Enhanced documentation and added test cases for different SMTP authentication scenarios.
Renamed environment variables from TEST_USER and TEST_PASS to TEST_SMTPAUTH_USER and TEST_SMTPAUTH_PASS for clarity and consistency in setting SMTP authentication credentials. This change ensures that the correct credentials are applied during tests.
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.55%. Comparing base (8faac3d) to head (c2d9104).
Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #335      +/-   ##
==========================================
- Coverage   86.85%   86.55%   -0.30%     
==========================================
  Files          26       26              
  Lines        2350     2351       +1     
==========================================
- Hits         2041     2035       -6     
- Misses        179      181       +2     
- Partials      130      135       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@james-d-elliott james-d-elliott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wneessen wneessen merged commit e854b21 into main Oct 16, 2024
32 checks passed
@wneessen wneessen deleted the bug/332_server-does-not-support-smtp-auth-error-when-using-localhost-in-v050 branch October 16, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"server does not support SMTP AUTH" error when using localhost in v0.5.0
2 participants