fix(admin): SMTP over implicit TLS, and rollbacks that survive
Server Deploy / deploy (push) Successful in 2m13s
Server Deploy / deploy (push) Successful in 2m13s
Two bugs, one symptom: signup created an account and a customer_user but no verification email ever arrived. net/smtp.SendMail only speaks STARTTLS. Against a port-465 server, which expects a TLS handshake immediately, it never delivers. The transport now wraps the connection before speaking SMTP on 465, exactly as sitesvc/internal/mail already did — the two are duplicated, so change both or consolidate into shared/. Also adds Date and Message-ID, whose absence gets a message scored as spam, and a 15s deadline on the conversation. The rollbacks ran on the HTTP request's context. A stalled mail server holds the request until the browser gives up, which cancels that context and turns both rollbacks into silent no-ops — stranding the exact rows they exist to remove. They now run detached with their own timeout, and log when they fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"crypto/rand"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"log"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -61,7 +62,18 @@ func CreateCustomerUser(ctx context.Context, accountID, email, password string)
|
||||
// worse than no row: it can never be signed in to, and it holds the
|
||||
// unique index on email, so the customer cannot sign up again with the
|
||||
// address they just used.
|
||||
_, _ = db.Admin("customer_users").DeleteOne(ctx, bson.M{"user_id": u.UserID})
|
||||
//
|
||||
// Deliberately NOT on ctx. ctx is the HTTP request's, and the most likely
|
||||
// reason we are here is that the mail server stalled until the browser
|
||||
// gave up — which cancels ctx and makes this delete a silent no-op,
|
||||
// stranding exactly the row it exists to remove. That happened in
|
||||
// production against a port-465 server.
|
||||
rbCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second)
|
||||
defer cancel()
|
||||
if _, dErr := db.Admin("customer_users").DeleteOne(rbCtx, bson.M{"user_id": u.UserID}); dErr != nil {
|
||||
log.Printf("signup: FAILED to roll back customer_user %s (%s) after mail error: %v",
|
||||
u.UserID, u.Email, dErr)
|
||||
}
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
@@ -123,8 +135,16 @@ func HandleSignup(c *gin.Context) {
|
||||
}
|
||||
|
||||
if err := CreateCustomerUser(ctx, acct.AccountID, email, body.Password); err != nil {
|
||||
// Roll the account back rather than strand one with no owner.
|
||||
_, _ = db.Admin("accounts").DeleteOne(ctx, bson.M{"account_id": acct.AccountID})
|
||||
// Roll the account back rather than strand one with no owner. Detached
|
||||
// from ctx for the same reason as the user rollback above: a stalled mail
|
||||
// server cancels the request, and a rollback that needs the request to
|
||||
// still be alive is a rollback that fails exactly when it is needed.
|
||||
log.Printf("signup: %s failed, rolling back account %s: %v", email, acct.AccountID, err)
|
||||
rbCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second)
|
||||
defer cancel()
|
||||
if _, dErr := db.Admin("accounts").DeleteOne(rbCtx, bson.M{"account_id": acct.AccountID}); dErr != nil {
|
||||
log.Printf("signup: FAILED to roll back account %s: %v", acct.AccountID, dErr)
|
||||
}
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "could not send the verification email"})
|
||||
return
|
||||
}
|
||||
|
||||
+110
-13
@@ -2,12 +2,22 @@
|
||||
package mail
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/tls"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"mime"
|
||||
"net"
|
||||
"net/smtp"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// timeout bounds the whole SMTP conversation. Without it a mail server that
|
||||
// accepts the connection and then stalls holds an HTTP request open until the
|
||||
// client gives up — and signup's rollback runs on that request's context.
|
||||
const timeout = 15 * time.Second
|
||||
|
||||
type Config struct {
|
||||
Host, Port, From, Username, Password string
|
||||
PublicURL string
|
||||
@@ -19,24 +29,111 @@ func Init(c Config) { cfg = c }
|
||||
|
||||
func Enabled() bool { return cfg.Host != "" && cfg.From != "" }
|
||||
|
||||
// send delivers one message.
|
||||
//
|
||||
// Port 465 is implicit TLS: the server expects a TLS handshake immediately, so
|
||||
// the connection is wrapped BEFORE any SMTP is spoken. Every other port gets
|
||||
// plaintext then STARTTLS if offered. `net/smtp.SendMail` only does the latter,
|
||||
// which is why it fails against a 465 mail server — this exact bug silently
|
||||
// stopped every admin email from being delivered.
|
||||
//
|
||||
// This mirrors sitesvc/internal/mail, which solved the same problem first. The
|
||||
// two are duplicated rather than shared; if you change the transport here,
|
||||
// change it there too, or consolidate both into shared/.
|
||||
func send(to, subject, body string) error {
|
||||
if !Enabled() {
|
||||
return fmt.Errorf("SMTP is not configured")
|
||||
}
|
||||
msg := strings.Join([]string{
|
||||
"From: " + cfg.From,
|
||||
"To: " + to,
|
||||
"Subject: " + subject,
|
||||
"MIME-Version: 1.0",
|
||||
"Content-Type: text/plain; charset=utf-8",
|
||||
"", body,
|
||||
}, "\r\n")
|
||||
|
||||
var auth smtp.Auth
|
||||
if cfg.Username != "" {
|
||||
auth = smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host)
|
||||
if strings.TrimSpace(to) == "" {
|
||||
return fmt.Errorf("smtp: no recipient")
|
||||
}
|
||||
return smtp.SendMail(cfg.Host+":"+cfg.Port, auth, cfg.From, []string{to}, []byte(msg))
|
||||
|
||||
addr := net.JoinHostPort(cfg.Host, cfg.Port)
|
||||
conn, err := net.DialTimeout("tcp", addr, timeout)
|
||||
if err != nil {
|
||||
return fmt.Errorf("smtp: dial %s: %w", addr, err)
|
||||
}
|
||||
_ = conn.SetDeadline(time.Now().Add(timeout))
|
||||
|
||||
if cfg.Port == "465" {
|
||||
conn = tls.Client(conn, &tls.Config{ServerName: cfg.Host})
|
||||
}
|
||||
|
||||
client, err := smtp.NewClient(conn, cfg.Host)
|
||||
if err != nil {
|
||||
conn.Close()
|
||||
return fmt.Errorf("smtp: client: %w", err)
|
||||
}
|
||||
defer client.Close()
|
||||
|
||||
if cfg.Port != "465" {
|
||||
if ok, _ := client.Extension("STARTTLS"); ok {
|
||||
if err := client.StartTLS(&tls.Config{ServerName: cfg.Host}); err != nil {
|
||||
return fmt.Errorf("smtp: starttls: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if cfg.Username != "" {
|
||||
if err := client.Auth(smtp.PlainAuth("", cfg.Username, cfg.Password, cfg.Host)); err != nil {
|
||||
return fmt.Errorf("smtp: auth: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if err := client.Mail(cfg.From); err != nil {
|
||||
return fmt.Errorf("smtp: mail from: %w", err)
|
||||
}
|
||||
if err := client.Rcpt(to); err != nil {
|
||||
return fmt.Errorf("smtp: rcpt %s: %w", to, err)
|
||||
}
|
||||
|
||||
w, err := client.Data()
|
||||
if err != nil {
|
||||
return fmt.Errorf("smtp: data: %w", err)
|
||||
}
|
||||
if _, err := w.Write(message(to, subject, body)); err != nil {
|
||||
return fmt.Errorf("smtp: write: %w", err)
|
||||
}
|
||||
if err := w.Close(); err != nil {
|
||||
return fmt.Errorf("smtp: close data: %w", err)
|
||||
}
|
||||
return client.Quit()
|
||||
}
|
||||
|
||||
// message builds the RFC 5322 envelope.
|
||||
//
|
||||
// Date and Message-ID are not decoration: a message without them is scored as
|
||||
// spam by most filters, which is its own way of "the email never arrived".
|
||||
// Header values are stripped of CR and LF so a crafted instance name cannot
|
||||
// inject extra headers.
|
||||
func message(to, subject, body string) []byte {
|
||||
var b strings.Builder
|
||||
b.WriteString("From: " + sanitizeHeader(cfg.From) + "\r\n")
|
||||
b.WriteString("To: " + sanitizeHeader(to) + "\r\n")
|
||||
b.WriteString("Date: " + time.Now().Format(time.RFC1123Z) + "\r\n")
|
||||
b.WriteString("Message-ID: " + messageID(cfg.From) + "\r\n")
|
||||
b.WriteString("Subject: " + mime.QEncoding.Encode("utf-8", sanitizeHeader(subject)) + "\r\n")
|
||||
b.WriteString("MIME-Version: 1.0\r\n")
|
||||
b.WriteString("Content-Type: text/plain; charset=utf-8\r\n")
|
||||
b.WriteString("\r\n")
|
||||
b.WriteString(body)
|
||||
return []byte(b.String())
|
||||
}
|
||||
|
||||
func messageID(from string) string {
|
||||
domain := "vantage.local"
|
||||
if at := strings.LastIndex(from, "@"); at >= 0 && at < len(from)-1 {
|
||||
domain = strings.Trim(from[at+1:], "<> ")
|
||||
}
|
||||
var buf [16]byte
|
||||
if _, err := rand.Read(buf[:]); err != nil {
|
||||
return fmt.Sprintf("<%d@%s>", time.Now().UnixNano(), domain)
|
||||
}
|
||||
return fmt.Sprintf("<%s@%s>", hex.EncodeToString(buf[:]), domain)
|
||||
}
|
||||
|
||||
func sanitizeHeader(v string) string {
|
||||
return strings.NewReplacer("\r", " ", "\n", " ").Replace(v)
|
||||
}
|
||||
|
||||
func SendVerification(to, token string) error {
|
||||
|
||||
Reference in New Issue
Block a user