Improve auth. provider interface

The authentication provider can now provide multiple authorization
identities associated with credentials. Protocols that support that
(e.g. JMAP, SASL) can let the client select the wanted identity.

internal/auth/external/externalauth.go

@@ -71,13 +71,14 @@ func (ea *ExternalAuth) Init(cfg *config.Map) error {
 	return nil
 }
 
-func (ea *ExternalAuth) CheckPlain(username, password string) bool {
+func (ea *ExternalAuth) AuthPlain(username, password string) ([]string, error) {
 	accountName, ok := auth.CheckDomainAuth(username, ea.perDomain, ea.domains)
 	if !ok {
-		return false
+		return nil, module.ErrUnknownCredentials
 	}
 
-	return AuthUsingHelper(ea.Log, ea.helperPath, accountName, password)
+	// TODO: Extend process protocol to support multiple authorization identities.
+	return []string{username}, AuthUsingHelper(ea.helperPath, accountName, password)
 }
 
 func init() {

internal/auth/external/helperauth.go

@@ -1,44 +1,38 @@
 package external
 
 import (
+	"fmt"
 	"io"
 	"os/exec"
-	"strings"
 
-	"github.com/foxcpp/maddy/internal/log"
+	"github.com/foxcpp/maddy/internal/module"
 )
 
-func AuthUsingHelper(l log.Logger, binaryPath, accountName, password string) bool {
+func AuthUsingHelper(binaryPath, accountName, password string) error {
 	cmd := exec.Command(binaryPath)
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
-		l.Println("failed to obtain stdin pipe for helper process:", err)
-		return false
+		return fmt.Errorf("helperauth: stdin init: %w", err)
 	}
 	if err := cmd.Start(); err != nil {
-		l.Println("failed to start helper process:", err)
-		return false
+		return fmt.Errorf("helperauth: process start: %w", err)
 	}
 	if _, err := io.WriteString(stdin, accountName+"\n"); err != nil {
-		l.Println("failed to write stdin of helper process:", err)
-		return false
+		return fmt.Errorf("helperauth: stdin write: %w", err)
 	}
 	if _, err := io.WriteString(stdin, password+"\n"); err != nil {
-		l.Println("failed to write stdin of helper process:", err)
-		return false
+		return fmt.Errorf("helperauth: stdin write: %w", err)
 	}
 	if err := cmd.Wait(); err != nil {
-		l.Debugln(err)
 		if exitErr, ok := err.(*exec.ExitError); ok {
 			// Exit code 1 is for authentication failure.
-			// Exit code 2 is for other errors.
-			if exitErr.ExitCode() == 2 {
-				l.Println(strings.TrimSpace(string(exitErr.Stderr)))
+			if exitErr.ExitCode() != 1 {
+				return fmt.Errorf("helperauth: %w: %v", err, string(exitErr.Stderr))
 			}
+			return module.ErrUnknownCredentials
 		} else {
-			l.Println("failed to wait for helper process:", err)
+			return fmt.Errorf("helperauth: process wait: %w", err)
 		}
-		return false
 	}
-	return true
+	return nil
 }

internal/auth/pam/module.go

@@ -61,29 +61,28 @@ func (a *Auth) Init(cfg *config.Map) error {
 	return nil
 }
 
-func (a *Auth) CheckPlain(username, password string) bool {
+func (a *Auth) AuthPlain(username, password string) ([]string, error) {
 	var accountName string
 	if a.expectAddress {
 		var err error
 		accountName, _, err = address.Split(username)
 		if err != nil {
-			return false
+			return nil, err
 		}
 	} else {
 		accountName = username
 	}
 
 	if a.useHelper {
-		return external.AuthUsingHelper(a.Log, a.helperPath, accountName, password)
+		if err := external.AuthUsingHelper(a.helperPath, accountName, password); err != nil {
+			return nil, err
+		}
 	}
 	err := runPAMAuth(accountName, password)
 	if err != nil {
-		if err == ErrInvalidCredentials {
-			a.Log.Println(err)
-		}
-		return false
+		return nil, err
 	}
-	return true
+	return []string{username}, nil
 }
 
 func init() {

internal/auth/shadow/module.go

@@ -67,43 +67,37 @@ func (a *Auth) Init(cfg *config.Map) error {
 	return nil
 }
 
-func (a *Auth) CheckPlain(username, password string) bool {
+func (a *Auth) AuthPlain(username, password string) ([]string, error) {
 	accountName, _, err := address.Split(username)
 	if err != nil {
-		return false
+		return nil, err
 	}
 
 	if a.useHelper {
-		return external.AuthUsingHelper(a.Log, a.helperPath, accountName, password)
+		return []string{username}, external.AuthUsingHelper(a.helperPath, accountName, password)
 	}
 
 	ent, err := Lookup(accountName)
 	if err != nil {
-		if err != ErrNoSuchUser {
-			a.Log.Error("lookup error", err, "username", username)
-		}
-		return false
+		return nil, err
 	}
 
 	if !ent.IsAccountValid() {
-		a.Log.Msg("account is expired", "username", username)
-		return false
+		return nil, fmt.Errorf("shadow: account is expired")
 	}
 
 	if !ent.IsPasswordValid() {
-		a.Log.Msg("password is expired", "username", username)
-		return false
+		return nil, fmt.Errorf("shadow: password is expired")
 	}
 
 	if err := ent.VerifyPassword(password); err != nil {
-		if err != ErrWrongPassword {
-			a.Log.Printf("%v", err)
+		if err == ErrWrongPassword {
+			return nil, module.ErrUnknownCredentials
 		}
-		a.Log.Msg("password verification failed", "username", username)
-		return false
+		return nil, err
 	}
 
-	return true
+	return []string{username}, nil
 }
 
 func init() {

internal/config/module/auth.go

@@ -6,7 +6,7 @@ import (
 )
 
 func AuthDirective(m *config.Map, node *config.Node) (interface{}, error) {
-	var provider module.AuthProvider
+	var provider module.PlainAuth
 	if err := ModuleFromNode(node.Args, node, m.Globals, &provider); err != nil {
 		return nil, err
 	}

internal/endpoint/imap/imap.go

@@ -32,7 +32,7 @@ type Endpoint struct {
 	addrs     []string
 	serv      *imapserver.Server
 	listeners []net.Listener
-	Auth      module.AuthProvider
+	Auth      module.PlainAuth
 	Store     module.Storage
 
 	updater     imapbackend.BackendUpdater
@@ -184,11 +184,14 @@ func (endp *Endpoint) Close() error {
 }
 
 func (endp *Endpoint) Login(connInfo *imap.ConnInfo, username, password string) (imapbackend.User, error) {
-	if !endp.Auth.CheckPlain(username, password) {
-		endp.Log.Msg("authentication failed", "username", username, "src_ip", connInfo.RemoteAddr)
+	_, err := endp.Auth.AuthPlain(username, password)
+	if err != nil {
+		endp.Log.Error("authentication failed", err, "username", username, "src_ip", connInfo.RemoteAddr)
 		return nil, imapbackend.ErrInvalidCredentials
 	}
 
+	// TODO: Wrap GetOrCreateUser and possibly implement INBOXES extension
+	// (though it is draft 00 for quite some time so it likely has no future).
 	return endp.Store.GetOrCreateUser(username)
 }
 

internal/endpoint/smtp/smtp.go

@@ -519,7 +519,7 @@ func (endp *Endpoint) wrapErr(msgId string, mangleUTF8 bool, err error) error {
 
 type Endpoint struct {
 	hostname  string
-	Auth      module.AuthProvider
+	Auth      module.PlainAuth
 	serv      *smtp.Server
 	name      string
 	addrs     []string
@@ -803,11 +803,14 @@ func (endp *Endpoint) Login(state *smtp.ConnectionState, username, password stri
 		return nil, endp.wrapErr("", true, err)
 	}
 
-	if !endp.Auth.CheckPlain(username, password) {
-		endp.Log.Msg("authentication failed", "username", username, "src_ip", state.RemoteAddr)
+	_, err := endp.Auth.AuthPlain(username, password)
+	if err != nil {
+		// TODO: Update fail2ban filters.
+		endp.Log.Error("authentication failed", err, "username", username, "src_ip", state.RemoteAddr)
 		return nil, errors.New("Invalid credentials")
 	}
 
+	// TODO: Pass valid identifies to SMTP pipeline.
 	return endp.newSession(false, username, password, state), nil
 }
 

internal/endpoint/smtp/smtp_test.go

@@ -27,7 +27,7 @@ const testMsg = "From: <sender@example.org>\r\n" +
 	"\r\n" +
 	"foobar\r\n"
 
-func testEndpoint(t *testing.T, modName string, auth module.AuthProvider, tgt module.DeliveryTarget, checks []module.Check, cfg []config.Node) *Endpoint {
+func testEndpoint(t *testing.T, modName string, auth module.PlainAuth, tgt module.DeliveryTarget, checks []module.Check, cfg []config.Node) *Endpoint {
 	t.Helper()
 
 	mod, err := New(modName, []string{"tcp://127.0.0.1:" + testPort})

internal/module/auth.go

@@ -1,7 +1,16 @@
 package module
 
-// AuthProvider is the interface implemented by modules providing authentication using
+import "errors"
+
+var (
+	// ErrUnknownCredentials should be returned by auth. provider if supplied
+	// credentials are valid for it but are not recognized (e.g. not found in
+	// used DB).
+	ErrUnknownCredentials = errors.New("unknown credentials")
+)
+
+// PlainAuth is the interface implemented by modules providing authentication using
 // username:password pairs.
-type AuthProvider interface {
-	CheckPlain(username, password string) bool
+type PlainAuth interface {
+	AuthPlain(username, password string) ([]string, error)
 }

internal/module/dummy.go

@@ -8,15 +8,15 @@ import (
 	"github.com/foxcpp/maddy/internal/config"
 )
 
-// Dummy is a struct that implements AuthProvider and DeliveryTarget
+// Dummy is a struct that implements PlainAuth and DeliveryTarget
 // interfaces but does nothing. Useful for testing.
 //
 // It is always registered under the 'dummy' name and can be used in both tests
 // and the actual server code (but the latter is kinda pointless).
 type Dummy struct{ instName string }
 
-func (d *Dummy) CheckPlain(_, _ string) bool {
-	return true
+func (d *Dummy) AuthPlain(username, _ string) ([]string, error) {
+	return []string{username}, nil
 }
 
 func (d *Dummy) Name() string {

internal/storage/sql/sql.go

@@ -3,7 +3,7 @@
 //
 // Interfaces implemented:
 // - module.StorageBackend
-// - module.AuthProvider
+// - module.PlainAuth
 // - module.DeliveryTarget
 package sql
 
@@ -420,21 +420,25 @@ func prepareUsername(username string) (string, error) {
 	return mbox + "@" + domain, nil
 }
 
-func (store *Storage) CheckPlain(username, password string) bool {
+func (store *Storage) AuthPlain(username, password string) ([]string, error) {
 	// TODO: Pass session context there.
-	defer trace.StartRegion(context.Background(), "sql/CheckPlain").End()
+	defer trace.StartRegion(context.Background(), "sql/AuthPlain").End()
 
 	accountName, err := prepareUsername(username)
 	if err != nil {
-		return false
+		return nil, err
 	}
 
 	password, err = precis.OpaqueString.CompareKey(password)
 	if err != nil {
-		return false
+		return nil, err
 	}
 
-	return store.Back.CheckPlain(accountName, password)
+	// TODO: Make go-imap-sql CheckPlain return an actual error.
+	if !store.Back.CheckPlain(accountName, password) {
+		return nil, module.ErrUnknownCredentials
+	}
+	return []string{username}, nil
 }
 
 func (store *Storage) GetOrCreateUser(username string) (backend.User, error) {