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

Fix SMTP AUTH LOGIN method for servers with uncommon success messages #95

Conversation

wneessen
Copy link
Owner

@wneessen wneessen commented Jan 7, 2023

This fixes #94 and basically reverts d0f0435. As James points out correctly in #94, we should not assume specific responses from the server. As long as the spec is followed and the server returns the correct SMTP code, we should not do our own magic.

I've also extended the getTestConnection method in client_test.go, so that we can specify more test environment options like TEST_PORT and TEST_TLS_SKIP_VERIFY. This was needed for testing with the ProtonMail Bridge which listens on a different port and has non-trusted certificates.

This fixes #94 and basically reverts d0f0435. As James points out correctly in #94, we should not assume specific responses from the server. As long as the spec is followed and the server returns the correct SMTP code, we should not do our own magic.

I've also extended the `getTestConnection` method in client_test.go, so that we can specify more test environment options like `TEST_PORT` and `TEST_TLS_SKIP_VERIFY`. This was needed for testing with the ProtonMail Bridge which listens on a different port and has non-trusted certificates.
@wneessen wneessen linked an issue Jan 7, 2023 that may be closed by this pull request
Removed "2.7.0 Authentication successful" challenge since this should never be sent with `more == true`.
@codecov-commenter
Copy link

Codecov Report

Merging #95 (39a9949) into main (3b3c1e6) will decrease coverage by 0.07%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
- Coverage   83.15%   83.07%   -0.08%     
==========================================
  Files          15       15              
  Lines        1549     1548       -1     
==========================================
- Hits         1288     1286       -2     
- Misses        186      187       +1     
  Partials       75       75              
Impacted Files Coverage Δ
auth/login.go 96.29% <66.66%> (-3.71%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@wneessen wneessen merged commit fb1d319 into main Jan 7, 2023
@wneessen wneessen deleted the 94-login-auth-next-handler-should-unconditionally-succeed-if-more-is-false branch January 7, 2023 10:57
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.

LOGIN AUTH next handler should unconditionally succeed if more is false
2 participants