Improve SMTP authentication and Fix user creation bugs (#16612)

* Improve SMTP authentication, Fix user creation bugs and add LDAP cert/key options

This PR has two parts:

Improvements for SMTP authentication:

* Default to use SMTPS if port is 465, and allow setting of force SMTPS.
* Always use STARTTLS if available
* Provide CRAM-MD5 mechanism
* Add options for HELO hostname disabling
* Add options for providing certificates and keys
* Handle application specific password response as a failed user login
instead of as a 500.

Close #16104

Fix creation of new users:

* A bug was introduced when allowing users to change usernames which
prevents the creation of external users.
* The LoginSource refactor also broke this page.

Close #16104

Signed-off-by: Andrew Thornton <art27@cantab.net>

5 files changed, 76 insertions, 37 deletions

diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go
index e99fc67901..f2acbb0d4b 100644
--- a/services/auth/source/ldap/source_search.go
+++ b/services/auth/source/ldap/source_search.go
@@ -8,6 +8,8 @@ package ldap
 import (
 	"crypto/tls"
 	"fmt"
+	"net"
+	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
@@ -103,26 +105,27 @@ func (ls *Source) findUserDN(l *ldap.Conn, name string) (string, bool) {
 	return userDN, true
 }
 
-func dial(ls *Source) (*ldap.Conn, error) {
-	log.Trace("Dialing LDAP with security protocol (%v) without verifying: %v", ls.SecurityProtocol, ls.SkipVerify)
+func dial(source *Source) (*ldap.Conn, error) {
+	log.Trace("Dialing LDAP with security protocol (%v) without verifying: %v", source.SecurityProtocol, source.SkipVerify)
 
-	tlsCfg := &tls.Config{
-		ServerName:         ls.Host,
-		InsecureSkipVerify: ls.SkipVerify,
+	tlsConfig := &tls.Config{
+		ServerName:         source.Host,
+		InsecureSkipVerify: source.SkipVerify,
 	}
-	if ls.SecurityProtocol == SecurityProtocolLDAPS {
-		return ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", ls.Host, ls.Port), tlsCfg)
+
+	if source.SecurityProtocol == SecurityProtocolLDAPS {
+		return ldap.DialTLS("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)), tlsConfig)
 	}
 
-	conn, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ls.Host, ls.Port))
+	conn, err := ldap.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)))
 	if err != nil {
-		return nil, fmt.Errorf("Dial: %v", err)
+		return nil, fmt.Errorf("error during Dial: %v", err)
 	}
 
-	if ls.SecurityProtocol == SecurityProtocolStartTLS {
-		if err = conn.StartTLS(tlsCfg); err != nil {
+	if source.SecurityProtocol == SecurityProtocolStartTLS {
+		if err = conn.StartTLS(tlsConfig); err != nil {
 			conn.Close()
-			return nil, fmt.Errorf("StartTLS: %v", err)
+			return nil, fmt.Errorf("error during StartTLS: %v", err)
 		}
 	}
 
diff --git a/services/auth/source/smtp/auth.go b/services/auth/source/smtp/auth.go
index 8edf4fca15..d797982da1 100644
--- a/services/auth/source/smtp/auth.go
+++ b/services/auth/source/smtp/auth.go
@@ -6,9 +6,11 @@ package smtp
 
 import (
 	"crypto/tls"
-	"errors"
 	"fmt"
+	"net"
 	"net/smtp"
+	"os"
+	"strconv"
 
 	"code.gitea.io/gitea/models"
 )
@@ -42,40 +44,62 @@ func (auth *loginAuthenticator) Next(fromServer []byte, more bool) ([]byte, erro
 
 // SMTP authentication type names.
 const (
-	PlainAuthentication = "PLAIN"
-	LoginAuthentication = "LOGIN"
+	PlainAuthentication   = "PLAIN"
+	LoginAuthentication   = "LOGIN"
+	CRAMMD5Authentication = "CRAM-MD5"
 )
 
 // Authenticators contains available SMTP authentication type names.
-var Authenticators = []string{PlainAuthentication, LoginAuthentication}
+var Authenticators = []string{PlainAuthentication, LoginAuthentication, CRAMMD5Authentication}
 
 // Authenticate performs an SMTP authentication.
 func Authenticate(a smtp.Auth, source *Source) error {
-	c, err := smtp.Dial(fmt.Sprintf("%s:%d", source.Host, source.Port))
+	tlsConfig := &tls.Config{
+		InsecureSkipVerify: source.SkipVerify,
+		ServerName:         source.Host,
+	}
+
+	conn, err := net.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)))
 	if err != nil {
 		return err
 	}
-	defer c.Close()
+	defer conn.Close()
 
-	if err = c.Hello("gogs"); err != nil {
-		return err
+	if source.UseTLS() {
+		conn = tls.Client(conn, tlsConfig)
+	}
+
+	client, err := smtp.NewClient(conn, source.Host)
+	if err != nil {
+		return fmt.Errorf("failed to create NewClient: %w", err)
 	}
+	defer client.Close()
 
-	if source.TLS {
-		if ok, _ := c.Extension("STARTTLS"); ok {
-			if err = c.StartTLS(&tls.Config{
-				InsecureSkipVerify: source.SkipVerify,
-				ServerName:         source.Host,
-			}); err != nil {
-				return err
+	if !source.DisableHelo {
+		hostname := source.HeloHostname
+		if len(hostname) == 0 {
+			hostname, err = os.Hostname()
+			if err != nil {
+				return fmt.Errorf("failed to find Hostname: %w", err)
 			}
-		} else {
-			return errors.New("SMTP server unsupports TLS")
+		}
+
+		if err = client.Hello(hostname); err != nil {
+			return fmt.Errorf("failed to send Helo: %w", err)
+		}
+	}
+
+	// If not using SMTPS, always use STARTTLS if available
+	hasStartTLS, _ := client.Extension("STARTTLS")
+	if !source.UseTLS() && hasStartTLS {
+		if err = client.StartTLS(tlsConfig); err != nil {
+			return fmt.Errorf("failed to start StartTLS: %v", err)
 		}
 	}
 
-	if ok, _ := c.Extension("AUTH"); ok {
-		return c.Auth(a)
+	if ok, _ := client.Extension("AUTH"); ok {
+		return client.Auth(a)
 	}
+
 	return models.ErrUnsupportedLoginType
 }
diff --git a/services/auth/source/smtp/source.go b/services/auth/source/smtp/source.go
index 3d80aea68a..39c9851ede 100644
--- a/services/auth/source/smtp/source.go
+++ b/services/auth/source/smtp/source.go
@@ -22,8 +22,10 @@ type Source struct {
 	Host           string
 	Port           int
 	AllowedDomains string `xorm:"TEXT"`
-	TLS            bool
+	ForceSMTPS     bool
 	SkipVerify     bool
+	HeloHostname   string
+	DisableHelo    bool
 
 	// reference to the loginSource
 	loginSource *models.LoginSource
@@ -51,7 +53,7 @@ func (source *Source) HasTLS() bool {
 
 // UseTLS returns if TLS is set
 func (source *Source) UseTLS() bool {
-	return source.TLS
+	return source.ForceSMTPS || source.Port == 465
 }
 
 // SetLoginSource sets the related LoginSource
diff --git a/services/auth/source/smtp/source_authenticate.go b/services/auth/source/smtp/source_authenticate.go
index 9bab86604b..e3dcd83222 100644
--- a/services/auth/source/smtp/source_authenticate.go
+++ b/services/auth/source/smtp/source_authenticate.go
@@ -28,12 +28,15 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
 	}
 
 	var auth smtp.Auth
-	if source.Auth == PlainAuthentication {
+	switch source.Auth {
+	case PlainAuthentication:
 		auth = smtp.PlainAuth("", login, password, source.Host)
-	} else if source.Auth == LoginAuthentication {
+	case LoginAuthentication:
 		auth = &loginAuthenticator{login, password}
-	} else {
-		return nil, errors.New("Unsupported SMTP auth type")
+	case CRAMMD5Authentication:
+		auth = smtp.CRAMMD5Auth(login, password)
+	default:
+		return nil, errors.New("unsupported SMTP auth type")
 	}
 
 	if err := Authenticate(auth, source); err != nil {
@@ -44,6 +47,10 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
 			strings.Contains(err.Error(), "Username and Password not accepted") {
 			return nil, models.ErrUserNotExist{Name: login}
 		}
+		if (ok && tperr.Code == 534) ||
+			strings.Contains(err.Error(), "Application-specific password required") {
+			return nil, models.ErrUserNotExist{Name: login}
+		}
 		return nil, err
 	}
 
diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go
index b78fa9217e..b45ea6ea12 100644
--- a/services/forms/auth_form.go
+++ b/services/forms/auth_form.go
@@ -50,6 +50,9 @@ type AuthenticationForm struct {
 	SecurityProtocol              int `binding:"Range(0,2)"`
 	TLS                           bool
 	SkipVerify                    bool
+	HeloHostname                  string
+	DisableHelo                   bool
+	ForceSMTPS                    bool
 	PAMServiceName                string
 	PAMEmailDomain                string
 	Oauth2Provider                string