MFA fix, added IP blacklist, update webmail client

internal/smtpserver/abuseguard_integration_test.go

@@ -0,0 +1,42 @@
+package smtpserver
+
+import (
+	"net/smtp"
+	"testing"
+)
+
+// TestRepeatedFailedAuthBlacklistsIP is the live-flow check for the abuseguard wiring
+// (see auth.go's authenticate): enough real failed AUTH PLAIN attempts over real TCP
+// against a real smtpserver.Backend should land the source IP in esrv_ip_blacklist,
+// exactly as internal/abuseguard's own unit tests confirm in isolation — this confirms
+// the actual auth.go call site is wired up, not just the abuseguard package itself.
+func TestRepeatedFailedAuthBlacklistsIP(t *testing.T) {
+	backend := newTestBackend(t)
+	sec, _ := backend.Cfg.NewSection("Security")
+	sec.NewKey("abuse_detection_enabled", "true")
+	sec.NewKey("abuse_failure_threshold", "3")
+	sec.NewKey("abuse_detection_window_minutes", "10")
+	sec.NewKey("abuse_blacklist_base_hours", "12")
+	sec.NewKey("abuse_blacklist_max_hours", "168")
+
+	addr := startTestServer(t, backend)
+
+	for i := 0; i < 3; i++ {
+		c, err := smtp.Dial(addr)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if authErr := c.Auth(smtp.PlainAuth("", "test@example.com", "wrongpassword", "127.0.0.1")); authErr == nil {
+			t.Fatal("expected auth failure")
+		}
+		c.Close()
+	}
+
+	blocked, err := backend.DB.IsIPBlacklisted("127.0.0.1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !blocked {
+		t.Fatal("expected 127.0.0.1 to be blacklisted after 3 failed AUTH attempts (threshold=3)")
+	}
+}

internal/smtpserver/auth.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/emersion/go-sasl"
 	"github.com/emersion/go-smtp"
+	"mailgoserver/internal/abuseguard"
 	"mailgoserver/internal/db"
 )
 
@@ -76,6 +77,7 @@ func (s *Session) authenticate(username, password string) error {
 	if err != nil {
 		s.backend.Logger.Error("Authentication error: %v", err)
 		_ = s.backend.DB.LogAuthAttempt("sender", username, s.peerIP, false, fmt.Sprintf("Authentication error: %v", err))
+		abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 		return s.failAuth(451, "Internal server error")
 	}
 	if sender != nil && db.CheckPassword(password, sender.PasswordHash) {
@@ -91,6 +93,7 @@ func (s *Session) authenticate(username, password string) error {
 		if merr != nil {
 			s.backend.Logger.Error("Mailbox authentication error: %v", merr)
 			_ = s.backend.DB.LogAuthAttempt("mailbox", username, s.peerIP, false, fmt.Sprintf("Authentication error: %v", merr))
+			abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 			return s.failAuth(451, "Internal server error")
 		}
 		if mbox != nil {
@@ -103,6 +106,7 @@ func (s *Session) authenticate(username, password string) error {
 	}
 
 	_ = s.backend.DB.LogAuthAttempt("sender", username, s.peerIP, false, fmt.Sprintf("Invalid credentials for %s", username))
+	abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 	return s.failAuth(535, "Authentication failed")
 }
 

internal/smtpserver/mailbox_rules_test.go

@@ -34,9 +34,13 @@ func TestBlockedSenderRejectedAtRcpt(t *testing.T) {
 	}
 }
 
-func TestAllowListBypassesSpamRejection(t *testing.T) {
+// TestAllowListBypassesSpamQuarantine confirms a zero reject threshold quarantines a
+// non-allow-listed sender's mail into Spam (still accepted at SMTP level — spam is
+// stored for review, not silently bounced), while an allow-listed sender's mail
+// skips scoring entirely and lands in INBOX as normal.
+func TestAllowListBypassesSpamQuarantine(t *testing.T) {
 	backend, mailboxID := newTestBackendWithMailbox(t)
-	// Force every non-allow-listed message to be rejected as spam.
+	// Force every non-allow-listed message to be quarantined as spam.
 	backend.Cfg.Section("Mailstore").Key("spam_reject_score").SetValue("0")
 
 	send := func(t *testing.T) error {
@@ -63,8 +67,12 @@ func TestAllowListBypassesSpamRejection(t *testing.T) {
 		return w.Close()
 	}
 
-	if err := send(t); err == nil {
-		t.Fatal("expected delivery to fail as spam with a zero reject threshold and no allow-list entry")
+	if err := send(t); err != nil {
+		t.Fatalf("expected delivery accepted (quarantined) with a zero reject threshold and no allow-list entry, got: %v", err)
+	}
+	spamMsgs, err := backend.DB.ListMessagesInFolder(mailboxID, "Spam")
+	if err != nil || len(spamMsgs) != 1 {
+		t.Fatalf("expected 1 quarantined message in Spam, got %d (err=%v)", len(spamMsgs), err)
 	}
 
 	if _, err := backend.DB.AddAllowBlockEntry(mailboxID, "allow", "test@example.com"); err != nil {
@@ -73,6 +81,10 @@ func TestAllowListBypassesSpamRejection(t *testing.T) {
 	if err := send(t); err != nil {
 		t.Fatalf("expected delivery to succeed once the sender is allow-listed, got: %v", err)
 	}
+	inboxMsgs, err := backend.DB.ListMessagesInFolder(mailboxID, "INBOX")
+	if err != nil || len(inboxMsgs) != 1 {
+		t.Fatalf("expected 1 message in INBOX once allow-listed (spam scoring skipped), got %d (err=%v)", len(inboxMsgs), err)
+	}
 }
 
 func TestFilterRuleDeleteDropsMessage(t *testing.T) {

internal/smtpserver/mailbox_spam_test.go

@@ -0,0 +1,85 @@
+package smtpserver
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/smtp"
+	"testing"
+)
+
+// fakeRspamd stands in for a real rspamd instance, always returning the fixed
+// score/action given — enough to exercise deliverLocally's rspamd branch without a
+// live rspamd deployment.
+func fakeRspamd(t *testing.T, score float64, action string) *httptest.Server {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		json.NewEncoder(w).Encode(map[string]any{"score": score, "action": action})
+	}))
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+func sendTestMessage(t *testing.T, addr, subject string) error {
+	t.Helper()
+	c, err := smtp.Dial(addr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+	if err := c.Auth(smtp.PlainAuth("", "test@example.com", "testpass123", "127.0.0.1")); err != nil {
+		t.Fatalf("auth: %v", err)
+	}
+	if err := c.Mail("test@example.com"); err != nil {
+		t.Fatalf("MAIL FROM: %v", err)
+	}
+	if err := c.Rcpt("inbox@example.com"); err != nil {
+		t.Fatalf("RCPT: %v", err)
+	}
+	w, err := c.Data()
+	if err != nil {
+		t.Fatal(err)
+	}
+	w.Write([]byte("Subject: " + subject + "\r\n\r\nhi"))
+	return w.Close()
+}
+
+// TestRspamdExplicitRejectActionStillHardRejects confirms rspamd's own "reject"
+// action still hard-rejects at SMTP time (unlike a bare score-threshold hit, which is
+// quarantined to Spam instead — see TestRspamdScoreThresholdQuarantinesInsteadOfRejecting).
+func TestRspamdExplicitRejectActionStillHardRejects(t *testing.T) {
+	backend, mailboxID := newTestBackendWithMailbox(t)
+	rspamd := fakeRspamd(t, 20, "reject")
+	backend.Cfg.Section("Rspamd").Key("enabled").SetValue("true")
+	backend.Cfg.Section("Rspamd").Key("url").SetValue(rspamd.URL)
+	addr := startTestServer(t, backend)
+
+	if err := sendTestMessage(t, addr, "hi"); err == nil {
+		t.Fatal("expected delivery to be hard-rejected when rspamd's action is \"reject\"")
+	}
+	inboxMsgs, _ := backend.DB.ListMessagesInFolder(mailboxID, "INBOX")
+	spamMsgs, _ := backend.DB.ListMessagesInFolder(mailboxID, "Spam")
+	if len(inboxMsgs) != 0 || len(spamMsgs) != 0 {
+		t.Fatalf("expected nothing stored anywhere for a hard reject, got INBOX=%d Spam=%d", len(inboxMsgs), len(spamMsgs))
+	}
+}
+
+// TestRspamdScoreThresholdQuarantinesInsteadOfRejecting confirms a bare rspamd score
+// over the configured threshold (action something other than "reject") is accepted
+// and quarantined into Spam, not bounced.
+func TestRspamdScoreThresholdQuarantinesInsteadOfRejecting(t *testing.T) {
+	backend, mailboxID := newTestBackendWithMailbox(t)
+	rspamd := fakeRspamd(t, 20, "add header")
+	backend.Cfg.Section("Rspamd").Key("enabled").SetValue("true")
+	backend.Cfg.Section("Rspamd").Key("url").SetValue(rspamd.URL)
+	backend.Cfg.Section("Rspamd").Key("reject_score").SetValue("15")
+	addr := startTestServer(t, backend)
+
+	if err := sendTestMessage(t, addr, "hi"); err != nil {
+		t.Fatalf("expected delivery accepted (quarantined), got: %v", err)
+	}
+	spamMsgs, err := backend.DB.ListMessagesInFolder(mailboxID, "Spam")
+	if err != nil || len(spamMsgs) != 1 {
+		t.Fatalf("expected 1 quarantined message in Spam, got %d (err=%v)", len(spamMsgs), err)
+	}
+}

internal/smtpserver/session.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/emersion/go-smtp"
 	"gopkg.in/ini.v1"
+	"mailgoserver/internal/abuseguard"
 	"mailgoserver/internal/db"
 	"mailgoserver/internal/dkim"
 	"mailgoserver/internal/mailstore"
@@ -122,6 +123,7 @@ func (s *Session) validateSenderAuthorization(mailFrom string) (accept, authoriz
 			return true, true, fmt.Sprintf("Sender authorized to send as %s", mailFrom)
 		}
 		_ = s.backend.DB.LogAuthAttempt("sender_validation", fmt.Sprintf("%s -> %s", sender.Email, mailFrom), s.peerIP, false, "")
+		abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 		return false, false, fmt.Sprintf("Sender %s not authorized to send as %s", sender.Email, mailFrom)
 	}
 
@@ -137,6 +139,7 @@ func (s *Session) validateSenderAuthorization(mailFrom string) (accept, authoriz
 			return true, true, fmt.Sprintf("Mailbox authorized to send as alias %s", mailFrom)
 		}
 		_ = s.backend.DB.LogAuthAttempt("mailbox_validation", fmt.Sprintf("%s -> %s", mbox.Email, mailFrom), s.peerIP, false, "")
+		abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 		return false, false, fmt.Sprintf("Mailbox %s not authorized to send as %s", mbox.Email, mailFrom)
 	}
 
@@ -153,6 +156,7 @@ func (s *Session) validateSenderAuthorization(mailFrom string) (accept, authoriz
 		return true, true, fmt.Sprintf("IP authorized for domain %s", fromDomain)
 	}
 	_ = s.backend.DB.LogAuthAttempt("ip", fmt.Sprintf("%s -> %s", s.peerIP, fromDomain), s.peerIP, false, fmt.Sprintf("IP %s not authorized for domain %s", s.peerIP, fromDomain))
+	abuseguard.RecordFailureAndMaybeBlacklist(s.backend.DB, s.backend.Cfg, s.backend.Logger, s.peerIP)
 	return false, false, fmt.Sprintf("Not authorized to send for domain %s", fromDomain)
 }
 
@@ -370,39 +374,60 @@ func (s *Session) deliverLocally(rcpts, types []string, signedContent, messageID
 	results := make([]relay.Result, 0, len(rcpts))
 	for i, rcpt := range rcpts {
 		mbox := s.localMailboxes[strings.ToLower(rcpt)]
+		folder := "INBOX"
+		markRead := false
 
 		// An explicit per-mailbox allow-list entry bypasses spam scoring entirely —
 		// the built-in heuristic and optional rspamd check both run regardless of each
 		// other (additive, not either/or), but neither runs at all once allow-listed.
+		spamGated := false
 		if allowed, _ := s.backend.DB.IsAllowed(mbox.ID, s.mailFrom); !allowed {
-			reject := heuristicScore >= rejectScore
-			if !reject && rspamdEnabled {
-				if score, action, err := mailstore.CheckRspamd(rspamdURL, []byte(signedContent), s.mailFrom, rcpt); err == nil {
-					if action == "reject" || score >= float64(rspamdRejectScore) {
-						reject = true
+			quarantine := heuristicScore >= rejectScore
+			hardReject := false
+			if rspamdEnabled {
+				if score, rAction, err := mailstore.CheckRspamd(rspamdURL, []byte(signedContent), s.mailFrom, rcpt); err == nil {
+					// rspamd's own "reject" action is a considered policy decision
+					// (DNSBL hit, greylisting, etc.) worth still hard-rejecting at
+					// SMTP time to avoid backscatter; a bare score threshold hit
+					// (from either scorer) is quarantined instead of rejected, so a
+					// false positive is recoverable from the Spam folder rather than
+					// silently bounced with no trace.
+					if rAction == "reject" {
+						hardReject = true
+					} else if score >= float64(rspamdRejectScore) {
+						quarantine = true
 					}
 				}
 				// rspamd unreachable/erroring must not block mail — errors are swallowed,
 				// the built-in heuristic above is still the baseline gate either way.
 			}
-			if reject {
+			if hardReject {
 				results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "550", ErrorMessage: "Message rejected as spam"})
 				continue
 			}
+			if quarantine {
+				folder = "Spam"
+				spamGated = true
+			}
 		}
 
-		action, err := s.backend.Mailstore.ApplyRules(mbox.ID, map[string]string{"from": s.mailFrom, "to": rcpt, "subject": subject})
-		if err != nil {
-			results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "450", ErrorMessage: err.Error()})
-			continue
-		}
-		if action.Drop {
-			results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: "Discarded by filter rule"})
-			continue
-		}
-		folder := "INBOX"
-		if action.Folder != "" {
-			folder = action.Folder
+		// Filter rules organize legitimate mail the recipient already trusts arriving
+		// in their INBOX — a quarantined message skips them entirely and always lands
+		// in Spam, rather than a rule accidentally routing spam back into view.
+		if !spamGated {
+			action, err := s.backend.Mailstore.ApplyRules(mbox.ID, map[string]string{"from": s.mailFrom, "to": rcpt, "subject": subject})
+			if err != nil {
+				results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: "450", ErrorMessage: err.Error()})
+				continue
+			}
+			if action.Drop {
+				results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: "Discarded by filter rule"})
+				continue
+			}
+			if action.Folder != "" {
+				folder = action.Folder
+			}
+			markRead = action.MarkRead
 		}
 
 		uid, err := s.backend.Mailstore.StoreMessage(mbox.ID, folder, []byte(signedContent), messageID, s.mailFrom, subject)
@@ -414,12 +439,16 @@ func (s *Session) deliverLocally(rcpts, types []string, signedContent, messageID
 			results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "failed", ErrorCode: errCode, ErrorMessage: errMsg})
 			continue
 		}
-		if action.MarkRead {
+		if markRead {
 			if err := s.backend.DB.SetMessageFlags(mbox.ID, uid, `\Seen`); err != nil {
 				s.backend.Logger.Error("mark_read rule failed to set flag for message %d: %v", uid, err)
 			}
 		}
-		results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: "Delivered to local mailbox"})
+		serverResponse := "Delivered to local mailbox"
+		if spamGated {
+			serverResponse = "Quarantined to Spam folder"
+		}
+		results = append(results, relay.Result{Recipient: rcpt, RecipientType: types[i], Status: "success", ServerResponse: serverResponse})
 	}
 	return results
 }