diff --git a/CHANGELOG.md b/CHANGELOG.md index df8be69a..d1ece98f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,12 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad CSP and would break the embedding this exists for. ### Fixed +- **SMTP email works with servers that offer AUTH LOGIN but not AUTH PLAIN.** + Calnode previously used PLAIN for every authenticated SMTP connection, which + failed against servers such as Microsoft 365 that advertise `LOGIN XOAUTH2` + after STARTTLS. It now prefers PLAIN when offered and uses LOGIN otherwise. + LOGIN requires TLS, and servers offering neither supported method return a + clear error. - **Rescheduling on the manage page works again.** Its slot list called an `esc()` helper that only ever existed on the booking page, so any day with availability threw before rendering and could not be rescheduled. The helper is now defined diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9a505ddf..77fcc2e0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -645,6 +645,12 @@ as the desired state: genuinely broken SMTP config. Explicit credentials state intent. `GET /v1/settings/email` returns the live `transport`, and the admin UI badges it, so filled-in SMTP fields are never mistaken for SMTP delivery. +- **SMTP authentication:** after the configured TLS handshake, if any, `smtp.go` + reads the server's advertised AUTH mechanisms. It uses PLAIN when + available, otherwise LOGIN if offered (as with some Microsoft 365 servers). + LOGIN refuses to send credentials without TLS; if neither mechanism is offered, + sending fails with an unsupported-authentication error. Authentication failures + omit the server's full response from logs to avoid exposing credential details. - **TCP relay:** `EMAIL_SMTP_CONNECT_HOST` / `EMAIL_SMTP_CONNECT_PORT` are loaded by `internal/config` and passed through `SMTPConfig` / `BuildMailer` at boot and settings reload. They change only the dial address; TLS and authentication keep the original SMTP host. Active overrides are logged. - **The dial must stay bounded.** `defaultSMTPTimeout` once applied only via `conn.SetDeadline`, which runs *after* the dial returns, leaving the dial itself bounded diff --git a/internal/mailer/smtp.go b/internal/mailer/smtp.go index b15e5511..5816ce7c 100644 --- a/internal/mailer/smtp.go +++ b/internal/mailer/smtp.go @@ -90,6 +90,52 @@ func newDialers(deadline time.Time, host string) (net.Dialer, tls.Dialer) { } } +// loginAuth implements the two-challenge AUTH LOGIN exchange. Credentials are +// only sent after the connection has been upgraded to TLS. +type loginAuth struct { + username string + password string + step int +} + +func (a *loginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) { + if !server.TLS { + return "", nil, errors.New("mailer: AUTH LOGIN requires TLS") + } + return "LOGIN", nil, nil +} + +func (a *loginAuth) Next(_ []byte, more bool) ([]byte, error) { + if !more { + return nil, nil + } + switch a.step { + case 0: + a.step++ + return []byte(a.username), nil + case 1: + a.step++ + return []byte(a.password), nil + default: + return nil, errors.New("mailer: unexpected AUTH LOGIN challenge") + } +} + +func (s *SMTP) authForMethods(methods string) (smtp.Auth, error) { + available := strings.Fields(strings.ToUpper(methods)) + for _, method := range available { + if method == "PLAIN" { + return smtp.PlainAuth("", s.username, s.password, s.host), nil + } + } + for _, method := range available { + if method == "LOGIN" { + return &loginAuth{username: s.username, password: s.password}, nil + } + } + return nil, errors.New("mailer: SMTP server does not offer AUTH PLAIN or LOGIN") +} + func (s *SMTP) Send(ctx context.Context, msg Message) error { addr := net.JoinHostPort(s.connectHost, s.connectPort) // Built before anything is dialed, so an unusable recipient costs no connection. @@ -150,7 +196,11 @@ func (s *SMTP) Send(ctx context.Context, msg Message) error { defer c.Close() if s.username != "" { - auth := smtp.PlainAuth("", s.username, s.password, s.host) + _, methods := c.Extension("AUTH") + auth, err := s.authForMethods(methods) + if err != nil { + return err + } if err := c.Auth(auth); err != nil { // Don't wrap err — SMTP auth responses can contain server-side // detail that may expose credential information in logs. diff --git a/internal/mailer/smtp_test.go b/internal/mailer/smtp_test.go index 1421172f..80d158d6 100644 --- a/internal/mailer/smtp_test.go +++ b/internal/mailer/smtp_test.go @@ -4,11 +4,64 @@ import ( "context" "errors" "net" + "net/smtp" "sync/atomic" "testing" "time" ) +func TestSMTPAuthForMethods(t *testing.T) { + s := &SMTP{host: "smtp.office365.com", username: "user@example.com", password: "secret"} + + // Office 365 advertises LOGIN and XOAUTH2, but not PLAIN. + auth, err := s.authForMethods("LOGIN XOAUTH2") + if err != nil { + t.Fatalf("select LOGIN: %v", err) + } + if _, ok := auth.(*loginAuth); !ok { + t.Fatalf("AUTH LOGIN XOAUTH2 selected %T; want AUTH LOGIN", auth) + } + + // Prefer the existing mechanism when a server supports both. + auth, err = s.authForMethods("LOGIN PLAIN") + if err != nil { + t.Fatalf("select PLAIN: %v", err) + } + if auth == nil { + t.Fatal("AUTH LOGIN PLAIN selected no authentication mechanism; want PLAIN") + } + mechanism, _, err := auth.Start(&smtp.ServerInfo{Name: s.host, TLS: true}) + if err != nil || mechanism != "PLAIN" { + t.Fatalf("AUTH LOGIN PLAIN selected %q (error: %v); want PLAIN", mechanism, err) + } + if _, err := s.authForMethods("XOAUTH2"); err == nil { + t.Fatal("XOAUTH2-only server should reject unsupported password authentication") + } +} + +func TestLoginAuthRequiresTLSAndAnswersChallenges(t *testing.T) { + auth := &loginAuth{username: "user@example.com", password: "secret"} + if _, _, err := auth.Start(&smtp.ServerInfo{TLS: false}); err == nil { + t.Fatal("AUTH LOGIN accepted an unencrypted connection") + } + mechanism, initial, err := auth.Start(&smtp.ServerInfo{TLS: true}) + if err != nil || mechanism != "LOGIN" || initial != nil { + t.Fatalf("Start = %q, %q, %v; want LOGIN and no initial response", mechanism, initial, err) + } + for _, want := range []string{"user@example.com", "secret"} { + response, err := auth.Next(nil, true) + if err != nil || string(response) != want { + t.Fatalf("challenge response = %q, %v; want %q", response, err, want) + } + } + if _, err := auth.Next(nil, true); err == nil { + t.Fatal("AUTH LOGIN accepted a third challenge") + } + if response, err := auth.Next(nil, false); err != nil || response != nil { + t.Fatalf("completed exchange = %q, %v; want no response", response, err) + } +} + // A server that accepts the TCP connection but never writes the initial SMTP greeting // reproduces the exact class of hang this test guards against: a port/TLS-mode mismatch // (e.g. STARTTLS spoken to an implicit-TLS port) or an unresponsive server, where the