diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index ad0765f..c662bdc 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -238,6 +238,38 @@ for the delivery worker. A `4.7.5` deferral means the destination published a TLS policy that could not be satisfied — that is deliberate; the message is retried rather than sent in the clear. +**"Delivery Delay Notification" with `dial tcp …:25: i/o timeout`.** This is +step 1 of the prerequisites, and it is the single most common reason a new +deployment cannot send anywhere: + +``` +Reason: dial tcp alt4.gmail-smtp-in.l.google.com:25: dial tcp 192.178.213.26:25: i/o timeout +``` + +A *timeout* rather than a refusal is the signature of a provider block — +nothing is answering because the packets never leave. Receiving still works, +and the queued messages keep retrying, so nothing is lost while you sort it +out. Two ways forward: + +1. Ask the provider to unblock outbound port 25 for the host. +2. Send through a relay, which needs no cooperation from them: + + ```bash + RELAY_HOST=smtp.your-provider.example + RELAY_PORT=587 + RELAY_USER=... + RELAY_PASS=... + ``` + + Delivery then goes to the relay for every destination, and the relay is + added to the published SPF record automatically — without that the relay's + IP is not authorised for your domain and everything it forwards fails SPF. + Credentials are only ever sent over STARTTLS; if the relay does not offer + it, the message is deferred rather than the password sent in the clear. + +Watch the backlog drain on the admin console's outbound queue screen, or with +`make preflight` for the port check itself. + **`/health` returns `X-LambdaMail-Health: degraded`.** The stack is running but a certificate needs attention: usually no certificate exists for `PRIMARY_MAIL_HOST` because no router was declared for it, or one is expiring diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index 45e617a..75fbe2d 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -165,9 +165,12 @@ func run(cfg config) { // The webmail reads and sends through this service because the folder, // UID, flag and blob logic already lives here; duplicating it in another // service would mean two implementations of the same IMAP semantics. + // WithLocalFiling is what puts a copy of every sent message in Sent and + // lets drafts be stored at all. Without it the composer's mail left the + // building with no trace in the sender's own mailbox. webmailUC := usecase.NewWebmailUseCase( postgres.NewWebmailRepository(pool), blobReader, submissionUC, authRepo, cfg.PrimaryMailHost, - ) + ).WithLocalFiling(blobs, messages) router := httppresentation.NewRouter(usecase.NewIngestReportsUseCase(reportRepo), func() error { return pool.Ping(ctx) }) if cfg.JwtSecret == "" { diff --git a/services/protocols/internal/application/usecase/inbound_system_message_test.go b/services/protocols/internal/application/usecase/inbound_system_message_test.go new file mode 100644 index 0000000..904eb5f --- /dev/null +++ b/services/protocols/internal/application/usecase/inbound_system_message_test.go @@ -0,0 +1,110 @@ +package usecase + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/google/uuid" + + "lambdamail/protocols/internal/application/port" + "lambdamail/protocols/internal/domain/valueobject" +) + +// junkingScanner files everything as spam, standing in for the real scanner +// scoring a bounce-shaped message with no SPF result. +type junkingScanner struct{ called bool } + +func (s *junkingScanner) Scan(_ context.Context, _ port.ScanInput) (*valueobject.ScanResult, error) { + s.called = true + return &valueobject.ScanResult{ + Verdict: valueobject.ScanVerdictSpamJunk, + Score: 12.0, + Symbols: map[string]float64{}, + HeadersToAdd: map[string]string{}, + }, nil +} + +// A delivery status notification this server wrote itself must never be +// spam-filtered. +// +// The report goes through the same inbound path as mail from the internet, but +// it is injected with no client IP, so SPF can only be "none" and the content +// looks exactly like a bounce. The result was that "your message to X was +// delayed" arrived in the sender's own Junk folder - the single notification +// they most need to see, filed where they will not look. +func TestSystemGeneratedMessageIsNotSpamFiltered(t *testing.T) { + blobs := &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "dsn", SizeBytes: 10}} + messages := &recordingMessageRepository{} + scanner := &junkingScanner{} + + uc := NewProcessInboundEmailUseCase(nil, blobs, messages) + uc.SetScanner(scanner) + + err := uc.Handle(context.Background(), ProcessInboundEmailInput{ + Sender: "postmaster@mail.example.test", + Recipients: []port.MailboxRecord{{ID: uuid.New(), MaxMessageBytes: 100000}}, + RecipientAddresses: []string{"user@example.test"}, + Body: bytes.NewReader([]byte("Subject: Delivery Delay Notification\r\n\r\ndelayed\r\n")), + SystemGenerated: true, + }) + if err != nil { + t.Fatalf("Handle: %v", err) + } + + if scanner.called { + t.Error("the scanner ran on a message this server generated itself") + } + if len(messages.persisted) != 1 { + t.Fatalf("want 1 persisted message, got %d", len(messages.persisted)) + } + if folder := messages.persisted[0].TargetFolderName; folder == "Junk" { + t.Errorf("a postmaster notification was filed as Junk") + } +} + +// The exemption must be narrow: anything actually received from a peer is +// still scanned, or marking a message "system generated" would be a way past +// the spam filter. +func TestOrdinaryMessageIsStillScanned(t *testing.T) { + blobs := &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "x", SizeBytes: 10}} + messages := &recordingMessageRepository{} + scanner := &junkingScanner{} + + uc := NewProcessInboundEmailUseCase(nil, blobs, messages) + uc.SetScanner(scanner) + + err := uc.Handle(context.Background(), ProcessInboundEmailInput{ + Sender: "stranger@spam.test", + Recipients: []port.MailboxRecord{{ID: uuid.New(), MaxMessageBytes: 100000}}, + RecipientAddresses: []string{"user@example.test"}, + Body: bytes.NewReader([]byte("Subject: buy pills\r\n\r\nspam\r\n")), + }) + if err != nil { + t.Fatalf("Handle: %v", err) + } + if !scanner.called { + t.Fatal("an ordinary inbound message skipped the scanner") + } + if messages.persisted[0].TargetFolderName != "Junk" { + t.Errorf("a message the scanner called spam was not filed as Junk, got %q", + messages.persisted[0].TargetFolderName) + } +} + +// A DSN announces itself as automatic so the far side's vacation responder +// does not answer it. RFC 3834 section 5 distinguishes the two values, and +// filters read the header. +func TestDsnDeclaresItselfAutoGenerated(t *testing.T) { + payload, loop := valueobject.BuildDsnReport( + valueobject.DsnActionDelayed, "mail.example.test", + "sender@example.test", "far@remote.test", "abc", "timeout", + ) + if loop { + t.Fatal("unexpected loop prevention") + } + if !strings.Contains(string(payload), "Auto-Submitted: auto-generated") { + t.Errorf("a delivery report is auto-generated, not auto-replied:\n%s", payload) + } +} diff --git a/services/protocols/internal/application/usecase/outbound_worker.go b/services/protocols/internal/application/usecase/outbound_worker.go index a33ab31..235fb11 100644 --- a/services/protocols/internal/application/usecase/outbound_worker.go +++ b/services/protocols/internal/application/usecase/outbound_worker.go @@ -256,6 +256,9 @@ func (w *OutboundWorkerUseCase) sendDsn(ctx context.Context, action valueobject. Recipients: targets, RecipientAddresses: addresses, Body: bytes.NewReader(dsnBytes), + // This server wrote this message a moment ago; scanning our own + // postmaster notification only ever files it as Junk. + SystemGenerated: true, }) } diff --git a/services/protocols/internal/application/usecase/process_inbound_email.go b/services/protocols/internal/application/usecase/process_inbound_email.go index 63085fc..a789b47 100644 --- a/services/protocols/internal/application/usecase/process_inbound_email.go +++ b/services/protocols/internal/application/usecase/process_inbound_email.go @@ -40,6 +40,16 @@ type ProcessInboundEmailInput struct { // produce a "none" verdict. ClientIP net.IP HeloDomain string + // SystemGenerated marks a message this server produced itself - a delivery + // status notification, for instance - rather than one accepted from a peer. + // + // Such a message must not be spam-scanned. It is injected without a client + // IP, so SPF can only come back "none", and its bounce-shaped content then + // scores highly: the effect was that a "your mail to X was delayed" warning + // landed in the sender's own Junk folder, which is precisely where the one + // notification they need to read must never go. It also has no envelope + // sender to greylist and nothing to quarantine. + SystemGenerated bool } // ProcessInboundEmailUseCase implements the inbound half of PLAN.md section @@ -196,7 +206,7 @@ func (uc *ProcessInboundEmailUseCase) Handle(ctx context.Context, input ProcessI } } - if uc.scanner != nil { + if uc.scanner != nil && !input.SystemGenerated { recipientAddr := "" if len(input.RecipientAddresses) > 0 { recipientAddr = input.RecipientAddresses[0] diff --git a/services/protocols/internal/application/usecase/webmail_compose_test.go b/services/protocols/internal/application/usecase/webmail_compose_test.go new file mode 100644 index 0000000..0ae2a1f --- /dev/null +++ b/services/protocols/internal/application/usecase/webmail_compose_test.go @@ -0,0 +1,114 @@ +package usecase + +import ( + "strings" + "testing" +) + +// Every one of these reproduces a way the composer's mail was being mangled or +// lost between the browser and the queue. + +func TestComposedMessageCarriesTheBody(t *testing.T) { + // The HTTP layer read a field called "body" while the composer had always + // sent "html", so the message that reached this function had an empty body + // and every mail sent from the webmail arrived blank. + payload := string(buildMimeMessage(ComposeInput{ + From: "me@example.test", + To: []string{"you@example.test"}, + Subject: "Hello", + HTML: "

Real content

", + }, []string{"you@example.test"}, "mail.example.test")) + + if !strings.Contains(payload, "Real content") { + t.Fatalf("the body is missing from the message:\n%s", payload) + } +} + +func TestHtmlIsSentAsHtml(t *testing.T) { + payload := string(buildMimeMessage(ComposeInput{ + From: "me@example.test", + To: []string{"you@example.test"}, + Subject: "Formatted", + HTML: "

Hello there

", + }, []string{"you@example.test"}, "mail.example.test")) + + if !strings.Contains(payload, "multipart/alternative") { + t.Errorf("want multipart/alternative for an HTML message, got:\n%s", payload) + } + if !strings.Contains(payload, "Content-Type: text/html; charset=utf-8") { + t.Errorf("the HTML part is not declared as text/html:\n%s", payload) + } + // A plain-text alternative must exist and must not be empty: an + // HTML-only multipart message scores badly with every spam filter. + if !strings.Contains(payload, "Content-Type: text/plain; charset=utf-8") { + t.Errorf("no plain-text alternative:\n%s", payload) + } + if !strings.Contains(payload, "Hello there") { + t.Errorf("the plain-text alternative did not get a readable fallback:\n%s", payload) + } + // text/plain must come first - least preferred first, RFC 2046 5.1.4. + if strings.Index(payload, "text/plain") > strings.Index(payload, "text/html") { + t.Error("text/plain must precede text/html in multipart/alternative") + } +} + +func TestPlainTextOnlyMessageStaysSimple(t *testing.T) { + payload := string(buildMimeMessage(ComposeInput{ + From: "me@example.test", + To: []string{"you@example.test"}, + Subject: "Plain", + Text: "just words", + }, []string{"you@example.test"}, "mail.example.test")) + + if strings.Contains(payload, "multipart") { + t.Errorf("a text-only message should not be multipart:\n%s", payload) + } + if !strings.Contains(payload, "just words") { + t.Errorf("body missing:\n%s", payload) + } +} + +func TestBccNeverAppearsInTheHeaders(t *testing.T) { + // Bcc belongs to the envelope alone. Leaking it into the headers would + // show every recipient who else was blind-copied. + input := ComposeInput{ + From: "me@example.test", + To: []string{"you@example.test"}, + Bcc: []string{"secret@example.test"}, + Subject: "Quiet", + Text: "hi", + } + recipients := normaliseRecipients( + append(append(append([]string{}, input.To...), input.Cc...), input.Bcc...)) + + // It does reach the envelope, or the blind copy is simply not delivered - + // which is what happened while Bcc was not read from the request at all. + found := false + for _, r := range recipients { + if r == "secret@example.test" { + found = true + } + } + if !found { + t.Fatalf("bcc recipient missing from the envelope: %v", recipients) + } + + payload := string(buildMimeMessage(input, recipients, "mail.example.test")) + headers, _, _ := strings.Cut(payload, "\r\n\r\n") + if strings.Contains(strings.ToLower(headers), "bcc") || + strings.Contains(headers, "secret@example.test") { + t.Errorf("bcc leaked into the headers:\n%s", headers) + } +} + +func TestHtmlToPlainTextIsReadable(t *testing.T) { + got := htmlToPlainText("

First line

Second & last


") + for _, want := range []string{"First line", "Second & last", "- one", "- two"} { + if !strings.Contains(got, want) { + t.Errorf("plain-text fallback missing %q, got:\n%s", want, got) + } + } + if strings.Contains(got, "<") || strings.Contains(got, "&") { + t.Errorf("markup survived into the plain-text part:\n%s", got) + } +} diff --git a/services/protocols/internal/application/usecase/webmail_mail.go b/services/protocols/internal/application/usecase/webmail_mail.go index 98dab87..718fc88 100644 --- a/services/protocols/internal/application/usecase/webmail_mail.go +++ b/services/protocols/internal/application/usecase/webmail_mail.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "log" "mime" "net/mail" "strings" @@ -31,6 +32,12 @@ type WebmailUseCase struct { submission *ProcessOutboundEmailUseCase auth port.AuthRepository localHost string + // blobStore and messages file the sender's own copy into Sent, and hold + // drafts. Both are optional: when either is nil the copy is skipped rather + // than the send failing, because a message that has already been queued + // for delivery must not be reported as an error. + blobStore port.BlobStorage + messages port.InboundMessageRepository } func NewWebmailUseCase( @@ -46,6 +53,13 @@ func NewWebmailUseCase( return &WebmailUseCase{repo: repo, blobs: blobs, submission: submission, auth: auth, localHost: localHost} } +// WithLocalFiling enables the Sent copy and draft storage. +func (uc *WebmailUseCase) WithLocalFiling(store port.BlobStorage, messages port.InboundMessageRepository) *WebmailUseCase { + uc.blobStore = store + uc.messages = messages + return uc +} + // MailboxIDFor resolves a session's address to its mailbox, which the event // stream needs so it can filter pushes per account. func (uc *WebmailUseCase) MailboxIDFor(ctx context.Context, address string) (string, error) { @@ -114,8 +128,16 @@ type ComposeInput struct { From string To []string Cc []string + // Bcc reaches the envelope but never the headers - that is the whole + // point of it. It used not to exist here at all, so a blind copy typed + // into the webmail was silently dropped and nobody received it. + Bcc []string Subject string - Body string + // Text is the plain-text body. HTML, when present, is what the composer + // produced; both are sent together as multipart/alternative so a reader + // that cannot or will not render HTML still has something to show. + Text string + HTML string } // ErrNoRecipients rejects a submission with nobody to deliver to, before it @@ -126,7 +148,10 @@ var ErrNoRecipients = errors.New("webmail: at least one recipient is required") // case the SMTP ports use, so webmail mail gets the identical treatment: // send limits, DKIM signing and the durable queue. func (uc *WebmailUseCase) Send(ctx context.Context, input ComposeInput) error { - recipients := normaliseRecipients(append(append([]string{}, input.To...), input.Cc...)) + // Bcc joins the envelope here and is left out of buildMimeMessage, which + // is what makes it blind. + recipients := normaliseRecipients( + append(append(append([]string{}, input.To...), input.Cc...), input.Bcc...)) if len(recipients) == 0 { return ErrNoRecipients } @@ -141,16 +166,111 @@ func (uc *WebmailUseCase) Send(ctx context.Context, input ComposeInput) error { payload := buildMimeMessage(input, recipients, uc.localHost) - return uc.submission.Submit(ctx, ProcessOutboundEmailInput{ + if err := uc.submission.Submit(ctx, ProcessOutboundEmailInput{ MailboxID: account.ID, SenderAddr: account.EmailAddress, DomainName: account.DomainName, Recipients: recipients, Body: bytes.NewReader(payload), MaxRecipientsPerHour: account.MaxRecipientsPerHour, - }) + }); err != nil { + return err + } + + // Only after the message is durably queued. Filing it first would leave a + // copy in Sent for a message that was then rejected by the send limit. + // + // A failure here is logged and swallowed: the mail is already on its way, + // and telling the composer the send failed would invite a duplicate. + if err := uc.fileLocalCopy(ctx, account.ID, account.EmailAddress, "Sent", payload); err != nil { + log.Printf("webmail: could not file the Sent copy for %s: %v", account.EmailAddress, err) + } + return nil } +// fileLocalCopy stores a message the user themselves composed into one of +// their own folders. It reuses the inbound persistence path so the copy gets a +// real IMAP UID and shows up over IMAP as well as in the webmail. +func (uc *WebmailUseCase) fileLocalCopy( + ctx context.Context, mailboxID uuid.UUID, address, folder string, payload []byte, +) error { + if uc.blobStore == nil || uc.messages == nil { + return nil + } + blob, err := uc.blobStore.Store(ctx, bytes.NewReader(payload)) + if err != nil { + return err + } + headers := ExtractMessageHeaders(payload) + _, err = uc.messages.PersistAll(ctx, []port.PersistInboundMessageInput{{ + MailboxID: mailboxID, + Blob: blob, + SenderAddress: address, + RecipientAddress: address, + TargetFolderName: folder, + Subject: headers.Subject, + Snippet: headers.Snippet, + FromDisplayName: headers.FromDisplayName, + MessageIDHeader: headers.MessageID, + HasAttachments: headers.HasAttachments, + }}) + return err +} + +// SaveDraft files the message being composed into the Drafts folder. +// +// Nothing did this before: the composer displayed "draft saved automatically" +// from a setTimeout, with no request behind it, and closing the tab lost the +// message. Drafts are whole RFC 5322 messages like any other, so they are +// readable over IMAP too. +func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput) (uint32, error) { + account, err := uc.auth.FindByAddress(ctx, input.From) + if err != nil { + return 0, err + } + if account == nil { + return 0, ErrNoSuchMailbox + } + if uc.blobStore == nil || uc.messages == nil { + return 0, ErrDraftsUnavailable + } + + // A draft has no envelope, so recipients may legitimately be empty here - + // unlike Send, this must not reject a half-written message. + recipients := normaliseRecipients( + append(append(append([]string{}, input.To...), input.Cc...), input.Bcc...)) + payload := buildMimeMessage(input, recipients, uc.localHost) + + blob, err := uc.blobStore.Store(ctx, bytes.NewReader(payload)) + if err != nil { + return 0, err + } + headers := ExtractMessageHeaders(payload) + uids, err := uc.messages.PersistAll(ctx, []port.PersistInboundMessageInput{{ + MailboxID: account.ID, + Blob: blob, + SenderAddress: account.EmailAddress, + RecipientAddress: account.EmailAddress, + TargetFolderName: "Drafts", + Subject: headers.Subject, + Snippet: headers.Snippet, + FromDisplayName: headers.FromDisplayName, + MessageIDHeader: headers.MessageID, + HasAttachments: headers.HasAttachments, + }}) + if err != nil { + return 0, err + } + if len(uids) == 0 { + return 0, nil + } + return uint32(uids[0]), nil +} + +// ErrDraftsUnavailable reports that this deployment cannot store drafts, +// rather than accepting one and dropping it. +var ErrDraftsUnavailable = errors.New("webmail: draft storage is not configured") + func normaliseRecipients(raw []string) []string { seen := map[string]bool{} out := []string{} @@ -172,13 +292,25 @@ func normaliseRecipients(raw []string) []string { // buildMimeMessage assembles the headers a receiver expects. Date and // Message-ID are set here because a message without them is scored as spam by // most receivers (PLAN.md section 5). +// +// Bcc recipients are deliberately absent from the headers: they are carried in +// the envelope only, which is what "blind" means. +// +// A message with HTML goes out as multipart/alternative with a plain-text part +// first, in ascending order of preference per RFC 2046 section 5.1.4. Sending +// the composer's HTML under a text/plain header - which is what this did while +// the body was being dropped anyway - shows the recipient raw markup. func buildMimeMessage(input ComposeInput, recipients []string, localHost string) []byte { var b strings.Builder b.WriteString("From: " + input.From + "\r\n") if len(input.To) > 0 { b.WriteString("To: " + strings.Join(input.To, ", ") + "\r\n") } else { - b.WriteString("To: " + strings.Join(recipients, ", ") + "\r\n") + to := normaliseRecipients(input.To) + if len(to) == 0 { + to = recipients + } + b.WriteString("To: " + strings.Join(to, ", ") + "\r\n") } if len(input.Cc) > 0 { b.WriteString("Cc: " + strings.Join(input.Cc, ", ") + "\r\n") @@ -187,16 +319,84 @@ func buildMimeMessage(input ComposeInput, recipients []string, localHost string) b.WriteString("Date: " + time.Now().Format(time.RFC1123Z) + "\r\n") b.WriteString(fmt.Sprintf("Message-ID: <%s@%s>\r\n", uuid.NewString(), localHost)) b.WriteString("MIME-Version: 1.0\r\n") - b.WriteString("Content-Type: text/plain; charset=utf-8\r\n") - b.WriteString("Content-Transfer-Encoding: 8bit\r\n") - b.WriteString("\r\n") - b.WriteString(strings.ReplaceAll(input.Body, "\n", "\r\n")) - if !strings.HasSuffix(input.Body, "\n") { - b.WriteString("\r\n") + + text := input.Text + if text == "" && input.HTML != "" { + text = htmlToPlainText(input.HTML) } + + if input.HTML == "" { + b.WriteString("Content-Type: text/plain; charset=utf-8\r\n") + b.WriteString("Content-Transfer-Encoding: 8bit\r\n\r\n") + b.WriteString(normaliseEOL(text)) + return []byte(b.String()) + } + + boundary := "=_alt_" + uuid.NewString() + b.WriteString("Content-Type: multipart/alternative; boundary=\"" + boundary + "\"\r\n\r\n") + + b.WriteString("--" + boundary + "\r\n") + b.WriteString("Content-Type: text/plain; charset=utf-8\r\n") + b.WriteString("Content-Transfer-Encoding: 8bit\r\n\r\n") + b.WriteString(normaliseEOL(text)) + + b.WriteString("--" + boundary + "\r\n") + b.WriteString("Content-Type: text/html; charset=utf-8\r\n") + b.WriteString("Content-Transfer-Encoding: 8bit\r\n\r\n") + b.WriteString(normaliseEOL(input.HTML)) + + b.WriteString("--" + boundary + "--\r\n") return []byte(b.String()) } +// normaliseEOL converts to CRLF and guarantees the trailing break a MIME part +// boundary must be preceded by. +func normaliseEOL(s string) string { + s = strings.ReplaceAll(strings.ReplaceAll(s, "\r\n", "\n"), "\n", "\r\n") + if !strings.HasSuffix(s, "\r\n") { + s += "\r\n" + } + return s +} + +// htmlToPlainText derives a readable fallback from composed HTML. +// +// Deliberately crude - block elements become line breaks, tags are dropped, +// the handful of entities a contenteditable actually emits are decoded. It +// exists so the text/plain part is not empty, which is what makes a +// multipart/alternative message look like spam to a filter. +func htmlToPlainText(html string) string { + replacer := strings.NewReplacer( + "
", "\n", "
", "\n", "
", "\n", + "

", "\n\n", "", "\n", "", "\n", "", "\n", + "
  • ", "- ", + ) + s := replacer.Replace(html) + + var out strings.Builder + depth := 0 + for _, r := range s { + switch { + case r == '<': + depth++ + case r == '>' && depth > 0: + depth-- + case depth == 0: + out.WriteRune(r) + } + } + + text := strings.NewReplacer( + " ", " ", "&", "&", "<", "<", ">", ">", """, "\"", "'", "'", + ).Replace(out.String()) + + // Collapse the runs of blank lines the block replacements leave behind. + for strings.Contains(text, "\n\n\n") { + text = strings.ReplaceAll(text, "\n\n\n", "\n\n") + } + return strings.TrimSpace(text) +} + // encodeHeaderValue RFC 2047-encodes a subject that is not plain ASCII, so an // accented subject does not arrive as mojibake. func encodeHeaderValue(value string) string { diff --git a/services/protocols/internal/domain/valueobject/dsn_bounce.go b/services/protocols/internal/domain/valueobject/dsn_bounce.go index 71490c2..a5ed47d 100644 --- a/services/protocols/internal/domain/valueobject/dsn_bounce.go +++ b/services/protocols/internal/domain/valueobject/dsn_bounce.go @@ -58,7 +58,12 @@ func BuildDsnReport(action DsnAction, reportingMTA string, envelopeFrom string, // RFC 3834 section 5: an automatic notification has to say so, or a // vacation responder on the other side answers it and the two bounce back // and forth forever. - sb.WriteString("Auto-Submitted: auto-replied\r\n") + // + // "auto-generated", not "auto-replied": the latter is for a message sent in + // response to a specific message (a vacation notice), while a DSN reports + // on a delivery attempt. Spam filters read this header, and the wrong value + // on a bounce-shaped message counts against it. + sb.WriteString("Auto-Submitted: auto-generated\r\n") sb.WriteString(fmt.Sprintf("Content-Type: multipart/report; report-type=delivery-status; boundary=\"%s\"\r\n\r\n", boundary)) // Part 1: Human-readable narrative diff --git a/services/protocols/internal/domain/valueobject/dsn_bounce_test.go b/services/protocols/internal/domain/valueobject/dsn_bounce_test.go index 1b501f5..1f60d3d 100644 --- a/services/protocols/internal/domain/valueobject/dsn_bounce_test.go +++ b/services/protocols/internal/domain/valueobject/dsn_bounce_test.go @@ -58,7 +58,7 @@ func TestBuildDsnReport_UsesTheConfiguredMailHost(t *testing.T) { func TestBuildDsnReport_MarksItselfAutomatic(t *testing.T) { payload, _ := BuildDsnReport(DsnActionDelayed, "mail.example.test", "sender@example.test", "recipient@remote.test", "job-1", "451 deferred") - if !strings.Contains(string(payload), "Auto-Submitted: auto-replied") { + if !strings.Contains(string(payload), "Auto-Submitted: auto-generated") { t.Errorf("the report is not marked as automatic: %s", payload) } } diff --git a/services/protocols/internal/presentation/http/mail_api.go b/services/protocols/internal/presentation/http/mail_api.go index 3b1ff71..29bc391 100644 --- a/services/protocols/internal/presentation/http/mail_api.go +++ b/services/protocols/internal/presentation/http/mail_api.go @@ -246,32 +246,80 @@ func (m *mailAPI) handleSend(w http.ResponseWriter, r *http.Request) { return } + input, ok := m.decodeCompose(w, r, session.Email) + if !ok { + return + } + + if err := m.useCase.Send(r.Context(), input); err != nil { + m.fail(w, err) + return + } + writeJSON(w, http.StatusAccepted, map[string]bool{"queued": true}) +} + +// handleSaveDraft stores the message being composed in the Drafts folder. +func (m *mailAPI) handleSaveDraft(w http.ResponseWriter, r *http.Request) { + session, ok := m.authenticate(w, r) + if !ok { + return + } + input, ok := m.decodeCompose(w, r, session.Email) + if !ok { + return + } + + uid, err := m.useCase.SaveDraft(r.Context(), input) + if err != nil { + m.fail(w, err) + return + } + writeJSON(w, http.StatusOK, map[string]any{"saved": true, "uid": uid}) +} + +// decodeCompose reads a composed message off the wire. +// +// "html" and "text" are both accepted, and both are new: the field was called +// "body" while the composer had always sent "html", so every message sent from +// the webmail was delivered with an empty body. "bcc" was likewise never read, +// so blind recipients were silently dropped. +func (m *mailAPI) decodeCompose( + w http.ResponseWriter, r *http.Request, sender string, +) (usecase.ComposeInput, bool) { var body struct { To []string `json:"to"` Cc []string `json:"cc"` + Bcc []string `json:"bcc"` Subject string `json:"subject"` - Body string `json:"body"` + Text string `json:"text"` + HTML string `json:"html"` + // Accepted so an older client - or a script written against the + // previous shape - still sends a readable message rather than a blank + // one. Only used when neither text nor html is present. + LegacyBody string `json:"body"` } if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxComposeBytes)).Decode(&body); err != nil { writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Invalid JSON") - return + return usecase.ComposeInput{}, false + } + + text := body.Text + if text == "" && body.HTML == "" { + text = body.LegacyBody } // The sender is the session's address, never a field in the request: // letting the body choose From is how a webmail becomes an open relay for // spoofed mail. - err := m.useCase.Send(r.Context(), usecase.ComposeInput{ - From: session.Email, + return usecase.ComposeInput{ + From: sender, To: body.To, Cc: body.Cc, + Bcc: body.Bcc, Subject: body.Subject, - Body: body.Body, - }) - if err != nil { - m.fail(w, err) - return - } - writeJSON(w, http.StatusAccepted, map[string]bool{"queued": true}) + Text: text, + HTML: body.HTML, + }, true } // fail maps domain errors onto status codes without leaking internals. @@ -285,6 +333,8 @@ func (m *mailAPI) fail(w http.ResponseWriter, err error) { writeError(w, http.StatusTooManyRequests, "SEND_LIMIT_EXCEEDED", "Hourly recipient limit reached") case errors.Is(err, usecase.ErrSenderNotOwned): writeError(w, http.StatusForbidden, "FORBIDDEN", "Sender address does not belong to this account") + case errors.Is(err, usecase.ErrDraftsUnavailable): + writeError(w, http.StatusServiceUnavailable, "DRAFTS_UNAVAILABLE", "Draft storage is not configured") default: writeError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "Request could not be processed") } diff --git a/services/protocols/internal/presentation/http/router.go b/services/protocols/internal/presentation/http/router.go index ca088b7..083c380 100644 --- a/services/protocols/internal/presentation/http/router.go +++ b/services/protocols/internal/presentation/http/router.go @@ -84,6 +84,7 @@ func (r *Router) registerRoutes() { r.mux.HandleFunc("/api/v1/mail/message/", r.handleMailMessage) r.mux.HandleFunc("/api/v1/mail/seen", r.handleMailSeen) r.mux.HandleFunc("/api/v1/mail/send", r.handleMailSend) + r.mux.HandleFunc("/api/v1/mail/draft", r.handleMailDraft) r.mux.HandleFunc("/api/v1/reports/dmarc", r.handleDmarcIngest) r.mux.HandleFunc("/api/v1/reports/tlsrpt", r.handleTlsRptIngest) } @@ -297,3 +298,13 @@ func (r *Router) handleMailSend(w http.ResponseWriter, req *http.Request) { r.mail.handleSend(w, req) } } + +func (r *Router) handleMailDraft(w http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPost { + writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "POST required") + return + } + if r.mailReady(w) { + r.mail.handleSaveDraft(w, req) + } +} diff --git a/services/webmail/messages/en.json b/services/webmail/messages/en.json index ba2f2c6..ffa0fb7 100644 --- a/services/webmail/messages/en.json +++ b/services/webmail/messages/en.json @@ -138,7 +138,9 @@ "bulletList": "Bulleted list", "compose": "Compose", "discard": "Discard", + "draftSaveFailed": "Draft could not be saved", "draftSaved": "Draft saved automatically", + "draftSaving": "Saving draft...", "drafts": "Drafts", "forward": "Forward", "hideCcBcc": "- Hide Cc/Bcc", diff --git a/services/webmail/messages/es.json b/services/webmail/messages/es.json index cd581ce..2e0b861 100644 --- a/services/webmail/messages/es.json +++ b/services/webmail/messages/es.json @@ -138,7 +138,9 @@ "bulletList": "Lista con viñetas", "compose": "Redactar", "discard": "Descartar", + "draftSaveFailed": "No se pudo guardar el borrador", "draftSaved": "Borrador guardado automáticamente", + "draftSaving": "Guardando borrador...", "drafts": "Borradores", "forward": "Reenviar", "hideCcBcc": "- Ocultar Cc/Cco", diff --git a/services/webmail/messages/pt-BR.json b/services/webmail/messages/pt-BR.json index c4e22fa..6a96c4c 100644 --- a/services/webmail/messages/pt-BR.json +++ b/services/webmail/messages/pt-BR.json @@ -138,7 +138,9 @@ "bulletList": "Lista com marcadores", "compose": "Escrever", "discard": "Descartar", + "draftSaveFailed": "Não foi possível salvar o rascunho", "draftSaved": "Rascunho salvo automaticamente", + "draftSaving": "Salvando rascunho...", "drafts": "Rascunhos", "forward": "Encaminhar", "hideCcBcc": "- Ocultar Cc/Cco", diff --git a/services/webmail/src/app/(user)/user/compose/page.tsx b/services/webmail/src/app/(user)/user/compose/page.tsx index bc8f787..c00b582 100644 --- a/services/webmail/src/app/(user)/user/compose/page.tsx +++ b/services/webmail/src/app/(user)/user/compose/page.tsx @@ -1,6 +1,6 @@ "use client"; -import React, { useEffect, useRef, useState, use } from "react"; +import React, { useCallback, useEffect, useRef, useState, use } from "react"; import Link from "next/link"; import { Send, @@ -21,6 +21,14 @@ import { useTranslations } from "../../../../i18n/provider"; import { Card, CardHeader, CardTitle } from "../../../../components/ui/Card"; import { Button } from "../../../../components/ui/Button"; +/** Splits a comma or semicolon separated field, dropping empty entries. */ +function splitAddresses(value: string): string[] { + return value + .split(/[,;]/) + .map((entry) => entry.trim()) + .filter(Boolean); +} + interface AttachedFile { id: string; name: string; @@ -45,6 +53,7 @@ export default function ComposePage({ const [sending, setSending] = useState(false); const [undoSeconds, setUndoSeconds] = useState(null); const [draftStatus, setDraftStatus] = useState(""); + const [sendError, setSendError] = useState(null); const editorRef = useRef(null); @@ -55,13 +64,39 @@ export default function ComposePage({ } }, []); + // Saves the draft for real. This used to be a setTimeout that set the words + // "draft saved automatically" and nothing else - no request, no storage - so + // closing the tab lost the message while the screen had just said otherwise. + const saveDraft = useCallback(async () => { + const html = editorRef.current?.innerHTML ?? ""; + if (!to && !subject && !html.trim()) return; + setDraftStatus(t("mail.draftSaving")); + try { + const res = await fetch("/api/v1/mail/draft", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + to: splitAddresses(to), + cc: splitAddresses(cc), + bcc: splitAddresses(bcc), + subject, + html, + }), + }); + setDraftStatus(res.ok ? t("mail.draftSaved") : t("mail.draftSaveFailed")); + } catch { + setDraftStatus(t("mail.draftSaveFailed")); + } + }, [to, cc, bcc, subject, t]); + + // Debounced so a draft is written after a pause in typing rather than on + // every keystroke. Skipped once the message is on its way out. useEffect(() => { + if (sending) return; if (!to && !subject) return; - const timer = setTimeout(() => { - setDraftStatus(t("mail.draftSaved")); - }, 2500); + const timer = setTimeout(() => void saveDraft(), 2500); return () => clearTimeout(timer); - }, [to, subject]); + }, [to, subject, cc, bcc, sending, saveDraft]); const executeFormat = (command: string, value: string | undefined = undefined) => { document.execCommand(command, false, value); @@ -104,20 +139,28 @@ export default function ComposePage({ const sendActualMessage = async (htmlBody: string) => { try { - await fetch("/api/v1/mail/send", { + const res = await fetch("/api/v1/mail/send", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - to: to.split(",").map((s) => s.trim()), - cc: cc ? cc.split(",").map((s) => s.trim()) : [], - bcc: bcc ? bcc.split(",").map((s) => s.trim()) : [], + to: splitAddresses(to), + cc: splitAddresses(cc), + bcc: splitAddresses(bcc), subject, html: htmlBody, - attachments: attachments.map((a) => a.name), }), }); + if (!res.ok) { + // Reporting success on a refused send is how a message silently + // disappears: the composer used to navigate away regardless. + const data = await res.json().catch(() => ({})); + setSendError(data.message ?? t("errors.serverError")); + setSending(false); + return; + } window.location.href = "/user/mail/sent"; } catch { + setSendError(t("errors.serverError")); setSending(false); } }; @@ -143,6 +186,12 @@ export default function ComposePage({ )} + {sendError && ( +
    + {sendError} +
    + )} + {undoSeconds !== null && (