From 55a91a37b71210f34f98f4d327c30308fe24399a Mon Sep 17 00:00:00 2001 From: "fox.cpp" Date: Fri, 28 Feb 2020 01:38:40 +0300 Subject: [PATCH] Revert authorization/authentication split Authentication provider module is responsible only for authentication. Nothing more. Access control (authorization) should be kept separate. --- docs/man/maddy-auth.5.scd | 21 +---- internal/auth/external/externalauth.go | 7 +- internal/auth/pam/module.go | 8 +- .../auth/plain_separate/plain_separate.go | 61 ++++--------- .../plain_separate/plain_separate_test.go | 65 ++++++-------- internal/auth/sasl.go | 90 ++++++------------- internal/auth/sasl_test.go | 66 ++++---------- internal/auth/shadow/module.go | 16 ++-- internal/config/module/interfaces.go | 8 -- internal/endpoint/imap/imap.go | 12 ++- internal/endpoint/smtp/smtp.go | 7 +- internal/module/auth.go | 20 +---- internal/module/dummy.go | 9 +- internal/storage/sql/sql.go | 10 +-- 14 files changed, 121 insertions(+), 279 deletions(-) diff --git a/docs/man/maddy-auth.5.scd b/docs/man/maddy-auth.5.scd index df52ac59..4f47efef 100644 --- a/docs/man/maddy-auth.5.scd +++ b/docs/man/maddy-auth.5.scd @@ -16,8 +16,7 @@ Most likely, you are going to use these modules with 'auth' directive of IMAP sql module described in *maddy-storage*(5) can also be used as a authentication backend. -The authorization identity is required to be a valid RFC 5321 e-mail address. -It is returned as the authorization identity. +The username is required to be a valid RFC 5321 e-mail address. # External authentication module (extauth) @@ -32,8 +31,6 @@ authentication is considered successful. If the status code is 1 - authentication is failed. If the status code is 2 - another unrelated error has happened. Additional information should be written to stderr. -The authorization identtity is the same as authorization identity. - ``` extauth { helper /usr/bin/ldap-helper @@ -79,13 +76,10 @@ maddy should be built with libpam build tag to use this module without go get -tags 'libpam' ... ``` -The authorization identtity is the same as authorization identity. - ``` pam { debug no use_helper no - expect_address yes } ``` @@ -109,7 +103,7 @@ privileges (e.g. when using system accounts). For 2, you need to make maddy-pam-helper binary setuid, see README.md in source tree for details. -TL;DR (assuming you have maddy group): +TL;DR (assuming you have the maddy group): ``` chown root:maddy /usr/lib/maddy/maddy-pam-helper chmod u+xs,g+x,o-x /usr/lib/maddy/maddy-pam-helper @@ -120,8 +114,6 @@ chmod u+xs,g+x,o-x /usr/lib/maddy/maddy-pam-helper Implements authentication by reading /etc/shadow. Alternatively it can be configured to use helper binary like extauth does. -The authorization identtity is the same as authorization identity. - ``` shadow { debug no @@ -175,14 +167,7 @@ How it works: 2. Each table specified with the 'user' directive looked up using normalized username. If match is not found in any table, authentication fails. 3. Each authentication provider specified with the 'pass' directive is tried. - First match wins same as in step 2. - -The set of authorization identities estabilished by plain_separate is the list of -strings found in 'user' tables. If there are no 'user' tables (only 'pass' -directives are specified), the list of authorization identities returned by -auth. providers is used. -*Note*: If a username table returns empty string, the authorization identity is -assumed to be the same as authentication identity. + If authentication with all providers fails - an error is returned. ## Configuration directives diff --git a/internal/auth/external/externalauth.go b/internal/auth/external/externalauth.go index 4a34e337..e7002a09 100644 --- a/internal/auth/external/externalauth.go +++ b/internal/auth/external/externalauth.go @@ -71,14 +71,13 @@ func (ea *ExternalAuth) Init(cfg *config.Map) error { return nil } -func (ea *ExternalAuth) AuthPlain(username, password string) ([]string, error) { +func (ea *ExternalAuth) AuthPlain(username, password string) error { accountName, ok := auth.CheckDomainAuth(username, ea.perDomain, ea.domains) if !ok { - return nil, module.ErrUnknownCredentials + return module.ErrUnknownCredentials } - // TODO: Extend process protocol to support multiple authorization identities. - return []string{username}, AuthUsingHelper(ea.helperPath, accountName, password) + return AuthUsingHelper(ea.helperPath, accountName, password) } func init() { diff --git a/internal/auth/pam/module.go b/internal/auth/pam/module.go index ca423408..c31b821d 100644 --- a/internal/auth/pam/module.go +++ b/internal/auth/pam/module.go @@ -58,17 +58,17 @@ func (a *Auth) Init(cfg *config.Map) error { return nil } -func (a *Auth) AuthPlain(username, password string) ([]string, error) { +func (a *Auth) AuthPlain(username, password string) error { if a.useHelper { if err := external.AuthUsingHelper(a.helperPath, username, password); err != nil { - return nil, err + return err } } err := runPAMAuth(username, password) if err != nil { - return nil, err + return err } - return []string{username}, nil + return nil } func init() { diff --git a/internal/auth/plain_separate/plain_separate.go b/internal/auth/plain_separate/plain_separate.go index 2e6e0c58..3f310f66 100644 --- a/internal/auth/plain_separate/plain_separate.go +++ b/internal/auth/plain_separate/plain_separate.go @@ -6,7 +6,6 @@ import ( "github.com/foxcpp/maddy/internal/config" "github.com/foxcpp/maddy/internal/log" "github.com/foxcpp/maddy/internal/module" - "golang.org/x/text/secure/precis" ) type Auth struct { @@ -54,56 +53,32 @@ func (a *Auth) Init(cfg *config.Map) error { return nil } -func (a *Auth) AuthPlain(username, password string) ([]string, error) { - key, err := precis.UsernameCaseMapped.CompareKey(username) - if err != nil { - return nil, err - } - - identities := make([]string, 0, 1) - if len(a.userTbls) != 0 { - for _, tbl := range a.userTbls { - repl, ok, err := tbl.Lookup(key) - if err != nil { - return nil, err - } - if !ok { - continue - } - if repl != "" { - identities = append(identities, repl) - } else { - identities = append(identities, key) - } - if a.onlyFirstID && len(identities) != 0 { - break - } +func (a *Auth) AuthPlain(username, password string) error { + ok := len(a.userTbls) == 0 + for _, tbl := range a.userTbls { + _, tblOk, err := tbl.Lookup(username) + if err != nil { + return err } - if len(identities) == 0 { - return nil, errors.New("plain_separate: unknown credentials") + if tblOk { + ok = true + break } } + if !ok { + return errors.New("user not found in tables") + } - var ( - lastErr error - ok bool - ) - for _, pass := range a.passwd { - passIDs, err := pass.AuthPlain(username, password) - if err != nil { + var lastErr error + for _, p := range a.passwd { + if err := p.AuthPlain(username, password); err != nil { lastErr = err continue } - if len(a.userTbls) == 0 { - identities = append(identities, passIDs...) - } - ok = true - } - if !ok { - return nil, lastErr - } - return identities, nil + return nil + } + return lastErr } func init() { diff --git a/internal/auth/plain_separate/plain_separate_test.go b/internal/auth/plain_separate/plain_separate_test.go index 0ffbea02..b8a622d9 100644 --- a/internal/auth/plain_separate/plain_separate_test.go +++ b/internal/auth/plain_separate/plain_separate_test.go @@ -2,7 +2,6 @@ package plain_separate import ( "errors" - "reflect" "testing" "github.com/emersion/go-sasl" @@ -10,19 +9,19 @@ import ( ) type mockAuth struct { - db map[string][]string + db map[string]bool } func (mockAuth) SASLMechanisms() []string { return []string{sasl.Plain, sasl.Login} } -func (m mockAuth) AuthPlain(username, _ string) ([]string, error) { - ids, ok := m.db[username] +func (m mockAuth) AuthPlain(username, _ string) error { + ok := m.db[username] if !ok { - return nil, errors.New("invalid creds") + return errors.New("invalid creds") } - return ids, nil + return nil } type mockTable struct { @@ -38,45 +37,39 @@ func TestPlainSplit_NoUser(t *testing.T) { a := Auth{ passwd: []module.PlainAuth{ mockAuth{ - db: map[string][]string{ - "user1": []string{"user1a", "user1b"}, + db: map[string]bool{ + "user1": true, }, }, }, } - ids, err := a.AuthPlain("user1", "aaa") + err := a.AuthPlain("user1", "aaa") if err != nil { t.Fatal("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) { - t.Fatal("Wrong ids returned:", ids) - } } func TestPlainSplit_NoUser_MultiPass(t *testing.T) { a := Auth{ passwd: []module.PlainAuth{ mockAuth{ - db: map[string][]string{ - "user2": []string{"user2a", "user2b"}, + db: map[string]bool{ + "user2": true, }, }, mockAuth{ - db: map[string][]string{ - "user1": []string{"user1a", "user1b"}, + db: map[string]bool{ + "user1": true, }, }, }, } - ids, err := a.AuthPlain("user1", "aaa") + err := a.AuthPlain("user1", "aaa") if err != nil { t.Fatal("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) { - t.Fatal("Wrong ids returned:", ids) - } } func TestPlainSplit_UserPass(t *testing.T) { @@ -84,31 +77,28 @@ func TestPlainSplit_UserPass(t *testing.T) { userTbls: []module.Table{ mockTable{ db: map[string]string{ - "user1": "user2", + "user1": "", }, }, }, passwd: []module.PlainAuth{ mockAuth{ - db: map[string][]string{ - "user2": []string{"user2a", "user2b"}, + db: map[string]bool{ + "user2": true, }, }, mockAuth{ - db: map[string][]string{ - "user1": []string{"user1a", "user1b"}, + db: map[string]bool{ + "user1": true, }, }, }, } - ids, err := a.AuthPlain("user1", "aaa") + err := a.AuthPlain("user1", "aaa") if err != nil { t.Fatal("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user2"}) { - t.Fatal("Wrong ids returned:", ids) - } } func TestPlainSplit_MultiUser_Pass(t *testing.T) { @@ -116,34 +106,31 @@ func TestPlainSplit_MultiUser_Pass(t *testing.T) { userTbls: []module.Table{ mockTable{ db: map[string]string{ - "userWH": "user1", + "userWH": "", }, }, mockTable{ db: map[string]string{ - "user1": "user2", + "user1": "", }, }, }, passwd: []module.PlainAuth{ mockAuth{ - db: map[string][]string{ - "user2": []string{"user2a", "user2b"}, + db: map[string]bool{ + "user2": true, }, }, mockAuth{ - db: map[string][]string{ - "user1": []string{"user1a", "user1b"}, + db: map[string]bool{ + "user1": true, }, }, }, } - ids, err := a.AuthPlain("user1", "aaa") + err := a.AuthPlain("user1", "aaa") if err != nil { t.Fatal("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user2"}) { - t.Fatal("Wrong ids returned:", ids) - } } diff --git a/internal/auth/sasl.go b/internal/auth/sasl.go index 5e92e0cb..7eecdc12 100644 --- a/internal/auth/sasl.go +++ b/internal/auth/sasl.go @@ -10,7 +10,6 @@ import ( modconfig "github.com/foxcpp/maddy/internal/config/module" "github.com/foxcpp/maddy/internal/log" "github.com/foxcpp/maddy/internal/module" - "golang.org/x/text/secure/precis" ) var ( @@ -39,85 +38,52 @@ func (s *SASLAuth) SASLMechanisms() []string { return mechs } -func (s *SASLAuth) AuthPlain(username, password string) ([]string, error) { +func (s *SASLAuth) AuthPlain(username, password string) error { if len(s.Plain) == 0 { - return nil, ErrUnsupportedMech + return ErrUnsupportedMech } var lastErr error - accounts := make([]string, 0, 1) for _, p := range s.Plain { - pAccs, err := p.AuthPlain(username, password) + err := p.AuthPlain(username, password) + if err == nil { + return nil + } if err != nil { lastErr = err continue } - - if s.OnlyFirstID { - return pAccs, nil - } - accounts = append(accounts, pAccs...) } - if len(accounts) == 0 { - return nil, fmt.Errorf("no auth. provider accepted creds, last err: %w", lastErr) - } - return accounts, nil -} - -func filterIdentity(accounts []string, identity string) ([]string, error) { - if identity == "" { - return accounts, nil - } - - matchFound := false - for _, acc := range accounts { - if precis.UsernameCaseMapped.Compare(acc, identity) { - accounts = []string{identity} - matchFound = true - break - } - } - if !matchFound { - return nil, errors.New("auth: invalid credentials") - } - return accounts, nil + return fmt.Errorf("no auth. provider accepted creds, last err: %w", lastErr) } // CreateSASL creates the sasl.Server instance for the corresponding mechanism. -// -// successCb will be called with the slice of authorization identities -// associated with credentials used. -// If it fails - authentication will fail too. -func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func([]string) error) sasl.Server { +func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func(identity string) error) sasl.Server { switch mech { case sasl.Plain: return sasl.NewPlainServer(func(identity, username, password string) error { - accounts, err := s.AuthPlain(username, password) - if err != nil { - s.Log.Error("authentication failed", err, "username", username, "identity", identity, "src_ip", remoteAddr) - return errors.New("auth: invalid credentials") + if identity == "" { + identity = username } - if len(accounts) == 0 { - accounts = []string{username} - } - accounts, err = filterIdentity(accounts, identity) + + err := s.AuthPlain(username, password) if err != nil { - s.Log.Error("not authorized", err, "username", username, "identity", identity, "src_ip", remoteAddr) + s.Log.Error("authentication failed", err, "username", username, "src_ip", remoteAddr) return errors.New("auth: invalid credentials") } - return successCb(accounts) + return successCb(identity) }) case sasl.Login: return sasl.NewLoginServer(func(username, password string) error { - accounts, err := s.AuthPlain(username, password) + err := s.AuthPlain(username, password) if err != nil { s.Log.Error("authentication failed", err, "username", username, "src_ip", remoteAddr) return errors.New("auth: invalid credentials") } - return successCb(accounts) + return successCb(username) }) } return FailingSASLServ{Err: ErrUnsupportedMech} @@ -126,24 +92,20 @@ func (s *SASLAuth) CreateSASL(mech string, remoteAddr net.Addr, successCb func([ // AddProvider adds the SASL authentication provider to its mapping by parsing // the 'auth' configuration directive. func (s *SASLAuth) AddProvider(m *config.Map, node *config.Node) error { - mod, err := modconfig.SASLAuthDirective(m, node) - if err != nil { + var any interface{} + if err := modconfig.ModuleFromNode(node.Args, node, m.Globals, &any); err != nil { return err } - saslAuth := mod.(module.SASLProvider) - for _, mech := range saslAuth.SASLMechanisms() { - switch mech { - case sasl.Login, sasl.Plain: - plainAuth, ok := saslAuth.(module.PlainAuth) - if !ok { - return m.MatchErr("auth: provider does not implement PlainAuth even though it reports PLAIN/LOGIN mechanism") - } - s.Plain = append(s.Plain, plainAuth) - default: - return m.MatchErr("auth: unknown SASL mechanism") - } + + hasAny := false + if plainAuth, ok := any.(module.PlainAuth); ok { + s.Plain = append(s.Plain, plainAuth) + hasAny = true } + if !hasAny { + return m.MatchErr("auth: specified module does not provide any SASL mechanism") + } return nil } diff --git a/internal/auth/sasl_test.go b/internal/auth/sasl_test.go index 8837dee1..10807cf7 100644 --- a/internal/auth/sasl_test.go +++ b/internal/auth/sasl_test.go @@ -3,28 +3,22 @@ package auth import ( "errors" "net" - "reflect" "testing" - "github.com/emersion/go-sasl" "github.com/foxcpp/maddy/internal/module" "github.com/foxcpp/maddy/internal/testutils" ) type mockAuth struct { - db map[string][]string + db map[string]bool } -func (mockAuth) SASLMechanisms() []string { - return []string{sasl.Plain, sasl.Login} -} - -func (m mockAuth) AuthPlain(username, _ string) ([]string, error) { - ids, ok := m.db[username] +func (m mockAuth) AuthPlain(username, _ string) error { + ok := m.db[username] if !ok { - return nil, errors.New("invalid creds") + return errors.New("invalid creds") } - return ids, nil + return nil } func TestCreateSASL(t *testing.T) { @@ -32,15 +26,15 @@ func TestCreateSASL(t *testing.T) { Log: testutils.Logger(t, "saslauth"), Plain: []module.PlainAuth{ &mockAuth{ - db: map[string][]string{ - "user1": []string{"user1a", "user1b"}, + db: map[string]bool{ + "user1": true, }, }, }, } t.Run("XWHATEVER", func(t *testing.T) { - srv := a.CreateSASL("XWHATEVER", &net.TCPAddr{}, func([]string) error { return nil }) + srv := a.CreateSASL("XWHATEVER", &net.TCPAddr{}, func(string) error { return nil }) _, _, err := srv.Next([]byte("")) if err == nil { t.Error("No error for XWHATEVER use") @@ -48,9 +42,10 @@ func TestCreateSASL(t *testing.T) { }) t.Run("PLAIN", func(t *testing.T) { - var ids []string - srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error { - ids = passed + srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(id string) error { + if id != "user1" { + t.Fatal("Wrong auth. identities passed to callback:", id) + } return nil }) @@ -58,15 +53,13 @@ func TestCreateSASL(t *testing.T) { if err != nil { t.Error("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user1a", "user1b"}) { - t.Error("Wrong auth. identities passed to callback:", ids) - } }) - t.Run("PLAIN with autorization identity", func(t *testing.T) { - var ids []string - srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error { - ids = passed + t.Run("PLAIN with authorization identity", func(t *testing.T) { + srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(id string) error { + if id != "user1a" { + t.Fatal("Wrong authorization identity passed:", id) + } return nil }) @@ -74,30 +67,5 @@ func TestCreateSASL(t *testing.T) { if err != nil { t.Error("Unexpected error:", err) } - if !reflect.DeepEqual(ids, []string{"user1a"}) { - t.Error("Wrong auth. identities passed to callback:", ids) - } - }) - - t.Run("PLAIN with wrong authorization identity", func(t *testing.T) { - srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error { - return nil - }) - - _, _, err := srv.Next([]byte("user1c\x00user1\x00aa")) - if err == nil { - t.Error("Next should fail") - } - }) - - t.Run("PLAIN with wrong authentication identity", func(t *testing.T) { - srv := a.CreateSASL("PLAIN", &net.TCPAddr{}, func(passed []string) error { - return nil - }) - - _, _, err := srv.Next([]byte("\x00user2\x00aa")) - if err == nil { - t.Error("Next should fail") - } }) } diff --git a/internal/auth/shadow/module.go b/internal/auth/shadow/module.go index 5812b70e..6135a036 100644 --- a/internal/auth/shadow/module.go +++ b/internal/auth/shadow/module.go @@ -66,32 +66,32 @@ func (a *Auth) Init(cfg *config.Map) error { return nil } -func (a *Auth) AuthPlain(username, password string) ([]string, error) { +func (a *Auth) AuthPlain(username, password string) error { if a.useHelper { - return []string{username}, external.AuthUsingHelper(a.helperPath, username, password) + return external.AuthUsingHelper(a.helperPath, username, password) } ent, err := Lookup(username) if err != nil { - return nil, err + return err } if !ent.IsAccountValid() { - return nil, fmt.Errorf("shadow: account is expired") + return fmt.Errorf("shadow: account is expired") } if !ent.IsPasswordValid() { - return nil, fmt.Errorf("shadow: password is expired") + return fmt.Errorf("shadow: password is expired") } if err := ent.VerifyPassword(password); err != nil { if err == ErrWrongPassword { - return nil, module.ErrUnknownCredentials + return module.ErrUnknownCredentials } - return nil, err + return err } - return []string{username}, nil + return nil } func init() { diff --git a/internal/config/module/interfaces.go b/internal/config/module/interfaces.go index cf8154c9..7ea1a30e 100644 --- a/internal/config/module/interfaces.go +++ b/internal/config/module/interfaces.go @@ -51,14 +51,6 @@ func StorageDirective(m *config.Map, node *config.Node) (interface{}, error) { return backend, nil } -func SASLAuthDirective(m *config.Map, node *config.Node) (interface{}, error) { - var provider module.SASLProvider - if err := ModuleFromNode(node.Args, node, m.Globals, &provider); err != nil { - return nil, err - } - return provider, nil -} - func TableDirective(m *config.Map, node *config.Node) (interface{}, error) { var tbl module.Table if err := ModuleFromNode(node.Args, node, m.Globals, &tbl); err != nil { diff --git a/internal/endpoint/imap/imap.go b/internal/endpoint/imap/imap.go index 808cee26..91a77968 100644 --- a/internal/endpoint/imap/imap.go +++ b/internal/endpoint/imap/imap.go @@ -123,8 +123,8 @@ func (endp *Endpoint) Init(cfg *config.Map) error { for _, mech := range endp.saslAuth.SASLMechanisms() { endp.serv.EnableAuth(mech, func(c imapserver.Conn) sasl.Server { - return endp.saslAuth.CreateSASL(mech, c.Info().RemoteAddr, func(ids []string) error { - return endp.openAccount(c, ids) + return endp.saslAuth.CreateSASL(mech, c.Info().RemoteAddr, func(identity string) error { + return endp.openAccount(c, identity) }) }) } @@ -199,8 +199,8 @@ func (endp *Endpoint) Close() error { return nil } -func (endp *Endpoint) openAccount(c imapserver.Conn, identities []string) error { - u, err := endp.Store.GetOrCreateUser(identities[0]) +func (endp *Endpoint) openAccount(c imapserver.Conn, identity string) error { + u, err := endp.Store.GetOrCreateUser(identity) if err != nil { return err } @@ -211,14 +211,12 @@ func (endp *Endpoint) openAccount(c imapserver.Conn, identities []string) error } func (endp *Endpoint) Login(connInfo *imap.ConnInfo, username, password string) (imapbackend.User, error) { - _, err := endp.saslAuth.AuthPlain(username, password) + err := endp.saslAuth.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) } diff --git a/internal/endpoint/smtp/smtp.go b/internal/endpoint/smtp/smtp.go index 3b71f5e3..e3ceb27b 100644 --- a/internal/endpoint/smtp/smtp.go +++ b/internal/endpoint/smtp/smtp.go @@ -269,8 +269,8 @@ func (endp *Endpoint) setConfig(cfg *config.Map) error { return auth.FailingSASLServ{Err: endp.wrapErr("", true, err)} } - return endp.saslAuth.CreateSASL(mech, state.RemoteAddr, func(ids []string) error { - c.SetSession(endp.newSession(false, ids[0], "", &state)) + return endp.saslAuth.CreateSASL(mech, state.RemoteAddr, func(id string) error { + c.SetSession(endp.newSession(false, id, "", &state)) return nil }) }) @@ -332,14 +332,13 @@ func (endp *Endpoint) Login(state *smtp.ConnectionState, username, password stri return nil, endp.wrapErr("", true, err) } - _, err := endp.saslAuth.AuthPlain(username, password) + err := endp.saslAuth.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 } diff --git a/internal/module/auth.go b/internal/module/auth.go index 06a58bc4..3daaab7d 100644 --- a/internal/module/auth.go +++ b/internal/module/auth.go @@ -12,23 +12,5 @@ var ( // PlainAuth is the interface implemented by modules providing authentication using // username:password pairs. type PlainAuth interface { - AuthPlain(username, password string) ([]string, error) -} - -// SASLProvider is the interface implemented by modules and used by protocol -// endpoints that rely on SASL framework for user authentication. -// -// This actual interface is only used to indicate that the module is a -// SASL-compatible auth. provider. For each unique value returned by -// SASLMechanisms, the module object should also implement the coresponding -// mechanism-specific interface. -// -// *Rationale*: There is no single generic interface that would handle any SASL -// mechanism while permiting the use of a credentials set estabilished once with -// multiple auth. providers at once. -// -// Per-mechanism interfaces: -// - PLAIN => PlainAuth -type SASLProvider interface { - SASLMechanisms() []string + AuthPlain(username, password string) error } diff --git a/internal/module/dummy.go b/internal/module/dummy.go index ee93003e..180bff5d 100644 --- a/internal/module/dummy.go +++ b/internal/module/dummy.go @@ -4,7 +4,6 @@ import ( "context" "github.com/emersion/go-message/textproto" - "github.com/emersion/go-sasl" "github.com/foxcpp/maddy/internal/buffer" "github.com/foxcpp/maddy/internal/config" ) @@ -16,12 +15,8 @@ import ( // and the actual server code (but the latter is kinda pointless). type Dummy struct{ instName string } -func (d *Dummy) SASLMechanisms() []string { - return []string{sasl.Plain, sasl.Login} -} - -func (d *Dummy) AuthPlain(username, _ string) ([]string, error) { - return []string{username}, nil +func (d *Dummy) AuthPlain(username, _ string) error { + return nil } func (d *Dummy) Name() string { diff --git a/internal/storage/sql/sql.go b/internal/storage/sql/sql.go index 435ff3bc..a453a562 100644 --- a/internal/storage/sql/sql.go +++ b/internal/storage/sql/sql.go @@ -420,25 +420,25 @@ func prepareUsername(username string) (string, error) { return mbox + "@" + domain, nil } -func (store *Storage) AuthPlain(username, password string) ([]string, error) { +func (store *Storage) AuthPlain(username, password string) error { // TODO: Pass session context there. defer trace.StartRegion(context.Background(), "sql/AuthPlain").End() accountName, err := prepareUsername(username) if err != nil { - return nil, err + return err } password, err = precis.OpaqueString.CompareKey(password) if err != nil { - return nil, err + return err } // TODO: Make go-imap-sql CheckPlain return an actual error. if !store.Back.CheckPlain(accountName, password) { - return nil, module.ErrUnknownCredentials + return module.ErrUnknownCredentials } - return []string{username}, nil + return nil } func (store *Storage) GetOrCreateUser(username string) (backend.User, error) {