diff --git a/admin/internal/auth/customer.go b/admin/internal/auth/customer.go index 463e859..52da2d2 100644 --- a/admin/internal/auth/customer.go +++ b/admin/internal/auth/customer.go @@ -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 } diff --git a/admin/internal/mail/mail.go b/admin/internal/mail/mail.go index bc6e6a4..f5d3f5d 100644 --- a/admin/internal/mail/mail.go +++ b/admin/internal/mail/mail.go @@ -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 {