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

[ugexe]

Description

Starting in v0.5.0 we started seeing a "server does not support SMTP AUTH" error when using a localhost client that we did not see in v0.4.4.

To Reproduce

Run the following with [email protected] and [email protected]. For me the former finishes, whereas the later gives me the error server does not support SMTP AUTH

package main

import (
	"fmt"
	"log"

	"github.com/wneessen/go-mail"
)

func main() {
	m := mail.NewMsg()
	m.From("[email protected]")
	m.To("[email protected]")
	m.Subject("test")
	m.SetBodyString(mail.TypeTextPlain, "hello")

	c, err := mail.NewClient(
		"localhost",
		mail.WithTLSPortPolicy(mail.NoTLS),
	)
	if err != nil {
		log.Fatal(err)
	}
	if err := c.DialAndSend(m); err != nil {
		log.Fatal(err)
	}

	fmt.Println("finished")
}

# v0.4.4
$ go run .
finished

# v0.5.0
$ go run .
2024/10/08 22:43:15 dial failed: server does not support SMTP AUTH
exit status 1

Expected behaviour

I'm expecting the behavior from v0.4.4 where smtp auth was not necessarily required.

Screenshots

No response

Attempted Fixes

No response

Additional context

This commit made some changes that look like they might have affects the default behavior. Notably in client.go line 830/834

v4.4 client.go auth() checks if c.smtpAuthType != "". If c.smtpAuthType defaults to "" then we would not have been entering the conditional on line 830 (and ultimately into the conditional with the "server does not support SMTP AUTH" error).

v5.0 client.go auth() checks if c.smtpAuthType != SMTPAuthCustom. If c.smtpAuthType defaults to "" then it would not match the value CUSTOM (i.e. SMTPAuthCustom) and would then enter the conditional on 1043 (and potentially into the conditional with the "server does not support SMTP AUTH" error).

[james-d-elliott]

Try the following adjustment:

package main

import (
	"fmt"
	"log"

	"github.com/wneessen/go-mail"
)

func main() {
	m := mail.NewMsg()
	m.From("[email protected]")
	m.To("[email protected]")
	m.Subject("test")
	m.SetBodyString(mail.TypeTextPlain, "hello")

	c, err := mail.NewClient(
		"localhost",
		mail.WithTLSPortPolicy(mail.NoTLS),
		mail.WithSMTPAuth(mail.SMTPAuthCustom)
	)
	if err != nil {
		log.Fatal(err)
	}
	if err := c.DialAndSend(m); err != nil {
		log.Fatal(err)
	}

	fmt.Println("finished")
}

[ugexe]

Yeah that works as expected and I'm fine with updating our code to do that. If this change in default behavior is intentional feel free to close this issue.

[james-d-elliott]

I don't believe it was intentional. I'm not an author of the project just someone with a vested interest and have made a few contributions. However as discussed here realistically it's actually a beneficial change. Specifically before SMTP servers which did not perform auth were implicitly accepted by the lib before, and this makes the acceptance of no-auth servers an explicit action.

When I get time I'll put together an answer to the followup question and it can be properly documented and tested.

[wneessen]

Thanks for raising this issue Nick, and thanks for helping out James! Helpful as usual.

The change was definetly not meant to be breaking, but I agree with James that it's beneficial. As some background: I've noticed during testing, that when I was setting the SMTPAuthType after the Client was already created, it would not adept the corresponding smtp.Auth method for that type, which could lead to the user speifying "From this point of time in the code I wanna use SMTP SCRAM Auth instead of SMTP LOGIN Auth" and therefore update the Client with SetSMTPAuth. But b/c the Client was initialized with the smtp.Auth method for SMTP LOGIN it would still use that and be in a weird mixed state. I fixed that behaviour but then ran into the issue that, if a custom smtp.Auth method was used by the user, it would not have a type associated. Therefore I introduced the SMTPAuthCustom type. That way we could always assume a valid smtp.Auth and type.

Again I agree with James, that the old behaviour was kind of weird especially if it would do no auth at all, if the user was expecting some. But failing when no auth is given is probably not the smart solution. It's probably a good idea to initialize the Clientwith SMTPNoAuth, so we have always a type set for the Client and then the user is free to override it with the WithSMTPAuth or SetSMTPAuth methods.

In any way, I am sorry for the disruption Nick and James and I am looking forward to your write-up.

[wneessen]

I have proposed #335 to address this issue.

I propose that 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 I introduced with v0.5.0 but also handle the edge-case that James described, where we would skip authentication without the user knowing.

[james-d-elliott]

I like the intent, it was exactly what I was going to suggest.

I will give it a once over again tomorrow I hope, it confused me when I looked today. I'll also run it through our CI integration suites.

[james-d-elliott]

Works well. See the CI run here: https://buildkite.com/authelia/authelia/builds/34221

[wneessen]

Thanks for testing and confirming James. I'll prepare a 0.5.1 release to fix the regression for tomorrow.

[james-d-elliott]

Here's the diff for your reference: authelia/authelia@d39d975

Previously we relied on the nil being supplied to SetSMTPAuthCustom by our NewOpportunisticSMTPAuth.