From 862749f8639e03ffd97b2527a9780602a9f2f838 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 7 Jan 2023 11:31:46 +0100 Subject: [PATCH 1/2] Fix SMTP AUTH LOGIN method for servers with uncommon success messages 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. --- auth/login.go | 12 +++--------- client_test.go | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/auth/login.go b/auth/login.go index 50cbc4fa..51f9179e 100644 --- a/auth/login.go +++ b/auth/login.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "net/smtp" - "strings" ) type loginAuth struct { @@ -23,10 +22,6 @@ const ( // ServerRespPassword represents the "Password:" response by the SMTP server ServerRespPassword = "Password:" - - // ServerRespAuthSuccess represents the "Authentication successful:" response that is - // by sent by some SMTP servers - ServerRespAuthSuccess = "Authentication successful" ) // LoginAuth returns an Auth that implements the LOGIN authentication @@ -70,10 +65,9 @@ func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) { return []byte(a.username), nil case ServerRespPassword: return []byte(a.password), nil + default: + return nil, fmt.Errorf("unexpected server response: %s", string(fromServer)) } } - if strings.HasSuffix(string(fromServer), ServerRespAuthSuccess) { - return nil, nil - } - return nil, fmt.Errorf("unexpected server response: %s", string(fromServer)) + return nil, nil } diff --git a/client_test.go b/client_test.go index d7ef11cf..b3bed52d 100644 --- a/client_test.go +++ b/client_test.go @@ -11,6 +11,7 @@ import ( "fmt" "net/smtp" "os" + "strconv" "strings" "testing" "time" @@ -1087,10 +1088,22 @@ func getTestConnection(auth bool) (*Client, error) { if th == "" { return nil, fmt.Errorf("no TEST_HOST set") } - c, err := NewClient(th) + tp := 25 + if tps := os.Getenv("TEST_PORT"); tps != "" { + tpi, err := strconv.Atoi(tps) + if err == nil { + tp = tpi + } + } + sv := false + if sve := os.Getenv("TEST_TLS_SKIP_VERIFY"); sve != "" { + sv = true + } + c, err := NewClient(th, WithPort(tp)) if err != nil { return c, err } + c.tlsconfig.InsecureSkipVerify = sv if auth { st := os.Getenv("TEST_SMTPAUTH_TYPE") if st != "" { From 39a9949e78bc1943d08217cc0cb9cdfbee1e6c7c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 7 Jan 2023 11:48:59 +0100 Subject: [PATCH 2/2] Fix auth_test.go accordingly Removed "2.7.0 Authentication successful" challenge since this should never be sent with `more == true`. --- auth/auth_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/auth/auth_test.go b/auth/auth_test.go index 75875afa..4df16cec 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -22,10 +22,10 @@ func TestAuth(t *testing.T) { authTests := []authTest{ { LoginAuth("user", "pass", "testserver"), - []string{"Username:", "Password:", "2.7.0 Authentication successful", "Invalid:"}, + []string{"Username:", "Password:", "Invalid:"}, "LOGIN", - []string{"", "user", "pass", "", ""}, - []bool{false, false, false, true}, + []string{"", "user", "pass", ""}, + []bool{false, false, true}, }, }