done
", + DraftUID: 42, + }); err != nil { + t.Fatalf("send: %v", err) + } + + if len(repo.expunged) != 1 || repo.expunged[0] != 42 { + t.Errorf("the draft was left in Drafts after sending: expunged %v, want [42]", repo.expunged) + } +} + +// A message composed from scratch has no draft behind it, and must not try to +// remove UID 0 - which is not a UID at all. +func TestSendingWithoutADraftExpungesNothing(t *testing.T) { + mailboxID := uuid.New() + repo := &stubWebmailRepo{mailboxID: mailboxID.String()} + uc := newSendableUseCase(t, mailboxID, repo) + + if err := uc.Send(context.Background(), ComposeInput{ + From: "me@example.test", To: []string{"you@example.test"}, Text: "hi", + }); err != nil { + t.Fatalf("send: %v", err) + } + if len(repo.expunged) != 0 { + t.Errorf("nothing should have been expunged, got %v", repo.expunged) + } +} + +// The Sent copy is a message the user wrote; it is not unread mail, and filing +// it as unread is what left the Sent folder with a badge nothing could clear. +func TestSentCopyIsFiledAsAlreadyRead(t *testing.T) { + mailboxID := uuid.New() + repo := &stubWebmailRepo{mailboxID: mailboxID.String()} + messages := &recordingMessageRepository{} + uc := newSendableUseCaseWith(t, mailboxID, repo, messages) + + if err := uc.Send(context.Background(), ComposeInput{ + From: "me@example.test", To: []string{"you@example.test"}, Text: "hi", + }); err != nil { + t.Fatalf("send: %v", err) + } + + if len(messages.persisted) != 1 { + t.Fatalf("want one filed copy, got %d", len(messages.persisted)) + } + if !messages.persisted[0].AlreadySeen { + t.Error("the Sent copy was filed as unread mail") + } +} + +// Likewise a draft: nothing ever opens one, so if it is filed unread its +// folder carries a badge forever. +func TestDraftIsFiledAsAlreadyRead(t *testing.T) { + mailboxID := uuid.New() + messages := &recordingMessageRepository{} + uc := (&WebmailUseCase{ + repo: &stubWebmailRepo{mailboxID: mailboxID.String()}, + auth: &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}}, + localHost: "mail.example.test", + }).WithLocalFiling( + &stubBlobStorage{ref: port.BlobRef{ID: uuid.New(), SHA256: "d", SizeBytes: 5}}, messages) + + if _, err := uc.SaveDraft(context.Background(), ComposeInput{ + From: "me@example.test", Subject: "half", + }, 0); err != nil { + t.Fatalf("save draft: %v", err) + } + if len(messages.persisted) != 1 || !messages.persisted[0].AlreadySeen { + t.Error("the draft was filed as unread mail") + } +} + +// --- deleting ------------------------------------------------------------ + +// Deleting from an ordinary folder moves the message to Trash. +func TestDeleteMovesToTrash(t *testing.T) { + mailboxID := uuid.New() + repo := &stubWebmailRepo{mailboxID: mailboxID.String()} + uc := &WebmailUseCase{ + repo: repo, + auth: &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}}, + localHost: "mail.example.test", + } + + if err := uc.Delete(context.Background(), "me@example.test", "inbox", 7); err != nil { + t.Fatalf("delete: %v", err) + } + if len(repo.movedToTrash) != 1 || repo.movedToTrash[0] != 7 { + t.Errorf("the message was not moved to Trash: %v", repo.movedToTrash) + } + if len(repo.expunged) != 0 { + t.Errorf("deleting from the inbox must not destroy the message: expunged %v", repo.expunged) + } +} + +// Deleting from Trash is the second press, and that one really removes it. +func TestDeleteFromTrashExpunges(t *testing.T) { + mailboxID := uuid.New() + repo := &stubWebmailRepo{mailboxID: mailboxID.String(), moveErr: port.ErrAlreadyInTrash} + uc := &WebmailUseCase{ + repo: repo, + auth: &stubAuthRepo{account: &port.MailboxAuth{ID: mailboxID, EmailAddress: "me@example.test"}}, + localHost: "mail.example.test", + } + + if err := uc.Delete(context.Background(), "me@example.test", "trash", 3); err != nil { + t.Fatalf("delete from trash: %v", err) + } + if len(repo.expunged) != 1 || repo.expunged[0] != 3 { + t.Errorf("emptying from Trash did not remove the message: %v", repo.expunged) + } +} diff --git a/services/protocols/internal/application/usecase/webmail_mail.go b/services/protocols/internal/application/usecase/webmail_mail.go index 23d1aa7..4f7f2d7 100644 --- a/services/protocols/internal/application/usecase/webmail_mail.go +++ b/services/protocols/internal/application/usecase/webmail_mail.go @@ -138,6 +138,13 @@ type ComposeInput struct { // that cannot or will not render HTML still has something to show. Text string HTML string + // DraftUID is the autosaved draft this message grew out of, or 0 when it + // was composed without one ever being stored. + // + // Sending used to ignore it entirely, so a message that had been autosaved + // went out and its draft stayed in Drafts: a duplicate of mail already on + // its way, which no screen in the webmail could then delete. + DraftUID uint32 } // ErrNoRecipients rejects a submission with nobody to deliver to, before it @@ -185,9 +192,46 @@ func (uc *WebmailUseCase) Send(ctx context.Context, input ComposeInput) error { 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) } + + // The draft this message grew out of has been superseded by the real + // thing. Swallowed like the Sent copy above and for the same reason: the + // mail is already queued, and failing the request now would only invite a + // second copy to be sent. + if input.DraftUID != 0 { + if mailboxID, idErr := uc.mailboxID(ctx, account.EmailAddress); idErr == nil { + if err := uc.repo.Expunge(ctx, mailboxID, "Drafts", input.DraftUID); err != nil { + log.Printf("webmail: could not discard the sent draft %d: %v", input.DraftUID, err) + } + } + } return nil } +// Delete removes one message the way a mail client does: from any ordinary +// folder it goes to Trash, and from Trash it is really expunged. +// +// The webmail had no delete at all. Expunge existed but was reachable only +// when an autosave superseded a draft, so anything the user wanted rid of - +// most visibly the draft left behind by a sent message - stayed put. +func (uc *WebmailUseCase) Delete(ctx context.Context, address, folder string, uid uint32) error { + mailboxID, err := uc.mailboxID(ctx, address) + if err != nil { + return err + } + + _, err = uc.repo.MoveToTrash(ctx, mailboxID, folder, uid) + switch { + case err == nil: + return nil + case errors.Is(err, port.ErrAlreadyInTrash), errors.Is(err, port.ErrNoTrashFolder): + // Already in Trash, or nowhere to move it to. Either way the second + // press is the one that removes it for good. + return uc.repo.Expunge(ctx, mailboxID, folder, uid) + default: + return err + } +} + // 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. @@ -213,6 +257,7 @@ func (uc *WebmailUseCase) fileLocalCopy( FromDisplayName: headers.FromDisplayName, MessageIDHeader: headers.MessageID, HasAttachments: headers.HasAttachments, + AlreadySeen: true, }}) return err } @@ -261,6 +306,7 @@ func (uc *WebmailUseCase) SaveDraft(ctx context.Context, input ComposeInput, rep FromDisplayName: headers.FromDisplayName, MessageIDHeader: headers.MessageID, HasAttachments: headers.HasAttachments, + AlreadySeen: true, }}) if err != nil { return 0, err diff --git a/services/protocols/internal/domain/entity/dmarc_report_test.go b/services/protocols/internal/domain/entity/dmarc_report_test.go index d8777a1..94386ba 100644 --- a/services/protocols/internal/domain/entity/dmarc_report_test.go +++ b/services/protocols/internal/domain/entity/dmarc_report_test.go @@ -72,7 +72,7 @@ func TestParseTlsRptReport_ValidJSON(t *testing.T) { { "policy": { "policy-type": "sts", - "domain": "example.test" + "policy-domain": "example.test" }, "summary": { "total-successful-session-count": 100, diff --git a/services/protocols/internal/domain/entity/tls_rpt_report.go b/services/protocols/internal/domain/entity/tls_rpt_report.go index f58ac00..fb2c9b8 100644 --- a/services/protocols/internal/domain/entity/tls_rpt_report.go +++ b/services/protocols/internal/domain/entity/tls_rpt_report.go @@ -34,7 +34,10 @@ type tlsRptJsonReport struct { Policies []struct { Policy struct { PolicyType string `json:"policy-type"` - Domain string `json:"domain"` + // RFC 8460 section 4.4 names this "policy-domain". Reading it as + // "domain" parsed every real report to a blank domain, so a stored + // report could not be attributed to the domain it was about. + Domain string `json:"policy-domain"` } `json:"policy"` Summary struct { TotalSuccessfulSessionCount int `json:"total-successful-session-count"` diff --git a/services/protocols/internal/domain/entity/tls_rpt_report_test.go b/services/protocols/internal/domain/entity/tls_rpt_report_test.go new file mode 100644 index 0000000..5664796 --- /dev/null +++ b/services/protocols/internal/domain/entity/tls_rpt_report_test.go @@ -0,0 +1,60 @@ +package entity + +import ( + "bytes" + "compress/gzip" + "testing" +) + +// This is a real report, as Google sends it daily: the gzipped JSON that +// arrives attached to a message addressed to the TLS-RPT rua. +const googleTlsRptReport = `{"organization-name":"Google Inc.",` + + `"date-range":{"start-datetime":"2026-08-19T00:00:00Z","end-datetime":"2026-08-19T23:59:59Z"},` + + `"contact-info":"smtp-tls-reporting@google.com","report-id":"2026-08-19T00:00:00Z_example.test",` + + `"policies":[{"policy":{"policy-type":"no-policy-found","policy-domain":"example.test"},` + + `"summary":{"total-successful-session-count":2,"total-failure-session-count":0}}]}` + +// The field is "policy-domain" (RFC 8460 section 4.4), not "domain". Reading +// the wrong name parsed every report to a blank domain, so a stored report +// could not be attributed to the domain it was about. +func TestTlsRptReportCarriesThePolicyDomain(t *testing.T) { + report, err := ParseTlsRptReport([]byte(googleTlsRptReport)) + if err != nil { + t.Fatalf("parse: %v", err) + } + if report.Domain != "example.test" { + t.Errorf("domain is %q, want example.test", report.Domain) + } + if report.OrganizationName != "Google Inc." { + t.Errorf("organization is %q", report.OrganizationName) + } + if len(report.Policies) != 1 { + t.Fatalf("want 1 policy, got %d", len(report.Policies)) + } + if report.Policies[0].SuccessCount != 2 { + t.Errorf("success count is %d, want 2", report.Policies[0].SuccessCount) + } + if report.Policies[0].PolicyType != "no-policy-found" { + t.Errorf("policy type is %q", report.Policies[0].PolicyType) + } +} + +// Reports arrive gzipped, which is how they land as a .json.gz attachment. +func TestTlsRptReportParsesGzippedPayload(t *testing.T) { + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + if _, err := gz.Write([]byte(googleTlsRptReport)); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + + report, err := ParseTlsRptReport(buf.Bytes()) + if err != nil { + t.Fatalf("parse gzipped: %v", err) + } + if report.Domain != "example.test" { + t.Errorf("domain is %q, want example.test", report.Domain) + } +} diff --git a/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go b/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go index e3f31fd..a37a1df 100644 --- a/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go +++ b/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go @@ -6,7 +6,9 @@ import ( "fmt" "log" "strings" + "time" + "github.com/google/uuid" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" @@ -99,23 +101,39 @@ func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessage return 0, fmt.Errorf("find folder %q for mailbox %s: %w", targetFolder, input.MailboxID, err) } + // A copy the user wrote themselves arrives already read, so it must not + // raise the unread counter. Incrementing unconditionally is what left the + // Sent and Drafts folders advertising unread mail forever: nothing ever + // opens your own outgoing copy, so the counter had no way back down. + unreadDelta := 1 + if input.AlreadySeen { + unreadDelta = 0 + } + var modseq int64 if err := tx.QueryRow(ctx, ` - UPDATE folders SET uid_next = uid_next + 1, highest_modseq = highest_modseq + 1, unread_count = unread_count + 1, total_count = total_count + 1 + UPDATE folders SET uid_next = uid_next + 1, highest_modseq = highest_modseq + 1, + unread_count = unread_count + $2, total_count = total_count + 1 WHERE id = $1 RETURNING highest_modseq - `, folderID).Scan(&modseq); err != nil { + `, folderID, unreadDelta).Scan(&modseq); err != nil { return 0, fmt.Errorf("advance uid_next, highest_modseq and folder counters: %w", err) } // email_messages.id defaults to gen_random_uuid() per the migration - no explicit value needed. - _, err = tx.Exec(ctx, ` + // id and received_at come back because message_flags is keyed on both: + // email_messages is partitioned by received_at, so the flag rows cannot be + // written without the same timestamp the row landed on. + var messageID uuid.UUID + var receivedAt time.Time + err = tx.QueryRow(ctx, ` INSERT INTO email_messages ( mailbox_id, folder_id, uid, modseq, blob_id, sender_address, recipient_addresses, size_bytes, subject, snippet, from_display_name, message_id_header, has_attachments, spf_result, dkim_result, dmarc_result ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) + RETURNING id, received_at `, input.MailboxID, folderID, uid, modseq, input.Blob.ID, input.SenderAddress, []string{input.RecipientAddress}, input.Blob.SizeBytes, nullIfEmpty(input.Subject), nullIfEmpty(input.Snippet), nullIfEmpty(input.FromDisplayName), @@ -129,11 +147,24 @@ func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessage // Sent copy and drafts have no authentication results by definition - // and it failed the whole insert. Sent stayed empty because that write // is deliberately non-fatal, and saving a draft reported an error. - nullIfEmpty(input.SPFResult), nullIfEmpty(input.DKIMResult), nullIfEmpty(input.DMARCResult)) + nullIfEmpty(input.SPFResult), nullIfEmpty(input.DKIMResult), nullIfEmpty(input.DMARCResult)). + Scan(&messageID, &receivedAt) if err != nil { return 0, fmt.Errorf("insert email_messages: %w", err) } + // The flags that describe what this copy is. Without \Seen on an outgoing + // copy the unread counters never settle; without \Draft an IMAP client + // shows a half-written message as ordinary mail and offers to reply to it. + for _, flag := range messageFlagsFor(input, targetFolder) { + if _, err := tx.Exec(ctx, ` + INSERT INTO message_flags (message_id, received_at, flag) + VALUES ($1, $2, $3) ON CONFLICT DO NOTHING + `, messageID, receivedAt, flag); err != nil { + return 0, fmt.Errorf("set flag %s: %w", flag, err) + } + } + if _, err := tx.Exec(ctx, `UPDATE message_blobs SET ref_count = ref_count + 1 WHERE id = $1`, input.Blob.ID); err != nil { return 0, fmt.Errorf("increment blob ref_count: %w", err) } @@ -151,3 +182,15 @@ func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessage return uid, nil } + +// messageFlagsFor returns the IMAP flags a newly filed copy is born with. +func messageFlagsFor(input port.PersistInboundMessageInput, folder string) []string { + flags := []string{} + if input.AlreadySeen { + flags = append(flags, `\Seen`) + } + if strings.EqualFold(folder, "Drafts") { + flags = append(flags, `\Draft`) + } + return flags +} diff --git a/services/protocols/internal/infrastructure/postgres/webmail_repository.go b/services/protocols/internal/infrastructure/postgres/webmail_repository.go index a26140c..8c054d9 100644 --- a/services/protocols/internal/infrastructure/postgres/webmail_repository.go +++ b/services/protocols/internal/infrastructure/postgres/webmail_repository.go @@ -40,13 +40,35 @@ func (r *WebmailRepository) FindMailboxIDByAddress(ctx context.Context, address } func (r *WebmailRepository) ListFolders(ctx context.Context, mailboxID string) ([]port.WebmailFolder, error) { + // The counts are derived from the messages themselves rather than read out + // of folders.unread_count/total_count. + // + // Those columns are a denormalised cache maintained by every write path, + // and once any one of them was wrong nothing brought it back: the number + // the user saw stayed wrong through a reload, a logout and a fresh login, + // because the wrongness was stored. Deriving here is authoritative and + // self-healing - a folder can only ever report what is actually in it. + // + // The columns stay maintained for IMAP's sake (STATUS answers from them), + // so this is a correction at the read side, not their removal. rows, err := r.pool.Query(ctx, ` - SELECT id::text, name, COALESCE(special_use, ''), unread_count, total_count - FROM folders WHERE mailbox_id = $1 - ORDER BY CASE special_use + SELECT f.id::text, f.name, COALESCE(f.special_use, ''), + COUNT(e.id) FILTER ( + WHERE NOT EXISTS (SELECT 1 FROM message_flags mf + WHERE mf.message_id = e.id + AND mf.received_at = e.received_at + AND mf.flag = '\Seen') + )::int AS unread_count, + COUNT(e.id)::int AS total_count + FROM folders f + LEFT JOIN email_messages e + ON e.folder_id = f.id AND e.expunged_at IS NULL + WHERE f.mailbox_id = $1 + GROUP BY f.id, f.name, f.special_use + ORDER BY CASE f.special_use WHEN 'inbox' THEN 0 WHEN 'drafts' THEN 1 WHEN 'sent' THEN 2 WHEN 'archive' THEN 3 WHEN 'junk' THEN 4 WHEN 'trash' THEN 5 - ELSE 6 END, name + ELSE 6 END, f.name `, mailboxID) if err != nil { return nil, fmt.Errorf("list folders: %w", err) @@ -224,7 +246,9 @@ func (r *WebmailRepository) Expunge( err = tx.QueryRow(ctx, ` SELECT e.id, f.id::text, NOT EXISTS (SELECT 1 FROM message_flags mf - WHERE mf.message_id = e.id AND mf.flag = '\Seen') + WHERE mf.message_id = e.id + AND mf.received_at = e.received_at + AND mf.flag = '\Seen') FROM email_messages e JOIN folders f ON f.id = e.folder_id WHERE e.mailbox_id = $1 AND `+folderCondition+` @@ -260,3 +284,110 @@ func (r *WebmailRepository) Expunge( return tx.Commit(ctx) } + +// MoveToTrash relocates one message into the mailbox's Trash folder and +// returns the UID it was given there. +// +// This is what "delete" means in a mail client, and the webmail had no way to +// do it at all: the only removal in the codebase was Expunge, reachable solely +// when an autosave superseded a draft. A message the user wanted rid of - the +// empty draft left behind by a sent message, most visibly - could not be +// removed from any screen. +// +// The message keeps its identity and its blob; only the folder and the UID +// change, because a UID is meaningful only within one folder. Deleting from +// Trash itself is Expunge's job, not this one's. +func (r *WebmailRepository) MoveToTrash( + ctx context.Context, mailboxID, folderName string, uid uint32, +) (uint32, error) { + tx, err := r.pool.Begin(ctx) + if err != nil { + return 0, err + } + defer tx.Rollback(ctx) + + var messageID uuid.UUID + var sourceFolderID string + var wasUnread bool + err = tx.QueryRow(ctx, ` + SELECT e.id, f.id::text, + NOT EXISTS (SELECT 1 FROM message_flags mf + WHERE mf.message_id = e.id + AND mf.received_at = e.received_at + AND mf.flag = '\Seen') + FROM email_messages e + JOIN folders f ON f.id = e.folder_id + WHERE e.mailbox_id = $1 AND `+folderCondition+` + AND e.uid = $3 AND e.expunged_at IS NULL + `, mailboxID, folderName, int64(uid)).Scan(&messageID, &sourceFolderID, &wasUnread) + if errors.Is(err, pgx.ErrNoRows) { + return 0, ErrMessageNotFound + } + if err != nil { + return 0, err + } + + // Deleting something already in Trash has no further destination, so the + // caller is told to expunge instead of the message being moved onto itself. + var trashFolderID string + var trashUID int64 + err = tx.QueryRow(ctx, ` + SELECT id::text, uid_next FROM folders + WHERE mailbox_id = $1 AND (special_use = 'trash' OR LOWER(name) = 'trash') + ORDER BY special_use IS NOT NULL DESC + LIMIT 1 + FOR UPDATE + `, mailboxID).Scan(&trashFolderID, &trashUID) + if errors.Is(err, pgx.ErrNoRows) { + return 0, port.ErrNoTrashFolder + } + if err != nil { + return 0, err + } + if trashFolderID == sourceFolderID { + return 0, port.ErrAlreadyInTrash + } + + var modseq int64 + if err := tx.QueryRow(ctx, ` + UPDATE folders SET uid_next = uid_next + 1, highest_modseq = highest_modseq + 1, + total_count = total_count + 1, unread_count = unread_count + $2 + WHERE id = $1 + RETURNING highest_modseq + `, trashFolderID, boolToInt(wasUnread)).Scan(&modseq); err != nil { + return 0, err + } + + if _, err := tx.Exec(ctx, ` + UPDATE email_messages SET folder_id = $2, uid = $3, modseq = $4 + WHERE id = $1 + `, messageID, trashFolderID, trashUID, modseq); err != nil { + return 0, err + } + + unreadDelta := 0 + if wasUnread { + unreadDelta = -1 + } + if _, err := tx.Exec(ctx, ` + UPDATE folders + SET total_count = GREATEST(0, total_count - 1), + unread_count = GREATEST(0, unread_count + $2), + highest_modseq = highest_modseq + 1 + WHERE id = $1 + `, sourceFolderID, unreadDelta); err != nil { + return 0, err + } + + if err := tx.Commit(ctx); err != nil { + return 0, err + } + return uint32(trashUID), nil +} + +func boolToInt(b bool) int { + if b { + return 1 + } + return 0 +} diff --git a/services/protocols/internal/infrastructure/postgres/webmail_repository_test.go b/services/protocols/internal/infrastructure/postgres/webmail_repository_test.go new file mode 100644 index 0000000..41cfb31 --- /dev/null +++ b/services/protocols/internal/infrastructure/postgres/webmail_repository_test.go @@ -0,0 +1,211 @@ +package postgres + +import ( + "context" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "lambdamail/protocols/internal/application/port" +) + +// These reproduce the counters the webmail was showing wrong: an unread badge +// that never came down however many messages were read, and a Sent folder that +// advertised unread mail the user had written themselves. + +// seedMailbox creates an isolated domain, mailbox and folder set for one test, +// so tests cannot see each other's counters. +func seedMailbox(t *testing.T, pool *pgxpool.Pool) string { + t.Helper() + ctx := context.Background() + domainID := uuid.New() + mbID := uuid.New() + name := "t" + uuid.NewString()[:8] + ".test" + + if _, err := pool.Exec(ctx, + `INSERT INTO domains (id, name, punycode_name, is_active) VALUES ($1,$2,$3,true)`, + domainID, name, name); err != nil { + t.Fatalf("seed domain: %v", err) + } + if _, err := pool.Exec(ctx, ` + INSERT INTO mailboxes (id, domain_id, local_part, email_address, password_hash, is_active) + VALUES ($1,$2,'user',$3,'x',true)`, + mbID, domainID, "user@"+name); err != nil { + t.Fatalf("seed mailbox: %v", err) + } + for _, f := range [][2]string{{"INBOX", "inbox"}, {"Sent", "sent"}, {"Drafts", "drafts"}, {"Trash", "trash"}} { + if _, err := pool.Exec(ctx, + `INSERT INTO folders (mailbox_id, name, special_use) VALUES ($1,$2,$3)`, + mbID, f[0], f[1]); err != nil { + t.Fatalf("seed folder %s: %v", f[0], err) + } + } + t.Cleanup(func() { + _, _ = pool.Exec(context.Background(), `DELETE FROM domains WHERE id = $1`, domainID) + }) + return mbID.String() +} + +// deliver files a message the way the inbound path does. +func deliver(t *testing.T, pool *pgxpool.Pool, mailboxID, folder, subject string, alreadySeen bool) uint32 { + t.Helper() + repo := NewInboundMessageRepository(pool) + blobID := storeTestBlob(t, pool) + uids, err := repo.PersistAll(context.Background(), []port.PersistInboundMessageInput{{ + MailboxID: uuid.MustParse(mailboxID), + Blob: port.BlobRef{ID: blobID, SHA256: uuid.NewString(), SizeBytes: 10}, + SenderAddress: "someone@example.test", + RecipientAddress: "user@example.test", + TargetFolderName: folder, + Subject: subject, + AlreadySeen: alreadySeen, + }}) + if err != nil { + t.Fatalf("persist: %v", err) + } + return uint32(uids[0]) +} + +func storeTestBlob(t *testing.T, pool *pgxpool.Pool) uuid.UUID { + t.Helper() + id := uuid.New() + if _, err := pool.Exec(context.Background(), ` + INSERT INTO message_blobs (id, content_sha256, storage_driver, storage_path, size_bytes, ref_count) + VALUES ($1, $2, 'local', $3, 10, 0)`, + id, strings.ReplaceAll(uuid.NewString()+uuid.NewString(), "-", "")[:64], "/dev/null/"+id.String()); err != nil { + t.Fatalf("seed blob: %v", err) + } + return id +} + +func folderByRole(t *testing.T, folders []port.WebmailFolder, role string) port.WebmailFolder { + t.Helper() + for _, f := range folders { + if f.SpecialUse == role { + return f + } + } + t.Fatalf("no %q folder in %v", role, folders) + return port.WebmailFolder{} +} + +// The bug the user reported: read a message, the unread number does not move, +// and it is still wrong after logging out and back in. +func TestUnreadCountFollowsWhatWasActuallyRead(t *testing.T) { + pool := testPool(t) + repo := NewWebmailRepository(pool) + mailboxID := seedMailbox(t, pool) + ctx := context.Background() + + uid := deliver(t, pool, mailboxID, "INBOX", "first", false) + deliver(t, pool, mailboxID, "INBOX", "second", false) + + folders, err := repo.ListFolders(ctx, mailboxID) + if err != nil { + t.Fatalf("list folders: %v", err) + } + if got := folderByRole(t, folders, "inbox").UnreadCount; got != 2 { + t.Fatalf("two unread messages delivered, folder reports %d", got) + } + + if err := repo.MarkSeen(ctx, mailboxID, "inbox", uid, true); err != nil { + t.Fatalf("mark seen: %v", err) + } + + folders, _ = repo.ListFolders(ctx, mailboxID) + if got := folderByRole(t, folders, "inbox").UnreadCount; got != 1 { + t.Errorf("after reading one of two, unread is %d, want 1", got) + } +} + +// A counter that has already drifted must correct itself, or the number stays +// wrong forever no matter what the user does. +func TestUnreadCountSelfHealsAfterDrift(t *testing.T) { + pool := testPool(t) + repo := NewWebmailRepository(pool) + mailboxID := seedMailbox(t, pool) + ctx := context.Background() + + deliver(t, pool, mailboxID, "INBOX", "only one", false) + + // Simulate the drift the old write path produced. + if _, err := pool.Exec(ctx, ` + UPDATE folders SET unread_count = 47, total_count = 99 + WHERE mailbox_id = $1 AND special_use = 'inbox'`, mailboxID); err != nil { + t.Fatalf("force drift: %v", err) + } + + folders, err := repo.ListFolders(ctx, mailboxID) + if err != nil { + t.Fatalf("list folders: %v", err) + } + inbox := folderByRole(t, folders, "inbox") + if inbox.UnreadCount != 1 { + t.Errorf("unread did not self-heal: got %d, want 1", inbox.UnreadCount) + } + if inbox.TotalCount != 1 { + t.Errorf("total did not self-heal: got %d, want 1", inbox.TotalCount) + } +} + +// A message the user wrote themselves is not unread mail. +func TestOwnSentCopyIsNotCountedAsUnread(t *testing.T) { + pool := testPool(t) + repo := NewWebmailRepository(pool) + mailboxID := seedMailbox(t, pool) + + deliver(t, pool, mailboxID, "Sent", "something I wrote", true) + + folders, err := repo.ListFolders(context.Background(), mailboxID) + if err != nil { + t.Fatalf("list folders: %v", err) + } + sent := folderByRole(t, folders, "sent") + if sent.UnreadCount != 0 { + t.Errorf("the Sent folder reports %d unread; a message you sent is not unread mail", sent.UnreadCount) + } + if sent.TotalCount != 1 { + t.Errorf("Sent total is %d, want 1", sent.TotalCount) + } +} + +// Deleting from an ordinary folder moves the message to Trash rather than +// destroying it, which is what every mail client does and what makes the +// action safe to offer. +func TestDeleteMovesToTrashThenExpungesFromTrash(t *testing.T) { + pool := testPool(t) + repo := NewWebmailRepository(pool) + mailboxID := seedMailbox(t, pool) + ctx := context.Background() + + uid := deliver(t, pool, mailboxID, "INBOX", "unwanted", false) + + trashUID, err := repo.MoveToTrash(ctx, mailboxID, "inbox", uid) + if err != nil { + t.Fatalf("move to trash: %v", err) + } + + inboxRows, _ := repo.ListMessages(ctx, mailboxID, "inbox", "", 50, 0) + if len(inboxRows) != 0 { + t.Errorf("the message is still in the inbox after being deleted: %d rows", len(inboxRows)) + } + trashRows, _ := repo.ListMessages(ctx, mailboxID, "trash", "", 50, 0) + if len(trashRows) != 1 { + t.Fatalf("the deleted message did not arrive in Trash: %d rows", len(trashRows)) + } + + folders, _ := repo.ListFolders(ctx, mailboxID) + if got := folderByRole(t, folders, "inbox").UnreadCount; got != 0 { + t.Errorf("inbox still counts the moved message as unread: %d", got) + } + + if err := repo.Expunge(ctx, mailboxID, "trash", trashUID); err != nil { + t.Fatalf("expunge from trash: %v", err) + } + trashRows, _ = repo.ListMessages(ctx, mailboxID, "trash", "", 50, 0) + if len(trashRows) != 0 { + t.Errorf("the message survived being emptied from Trash: %d rows", len(trashRows)) + } +} diff --git a/services/protocols/internal/presentation/http/mail_api.go b/services/protocols/internal/presentation/http/mail_api.go index 9ac0413..f595e64 100644 --- a/services/protocols/internal/presentation/http/mail_api.go +++ b/services/protocols/internal/presentation/http/mail_api.go @@ -259,6 +259,37 @@ func (m *mailAPI) handleSend(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusAccepted, map[string]bool{"queued": true}) } +// handleDelete removes one message: to Trash from anywhere else, and for good +// from Trash itself. +func (m *mailAPI) handleDelete(w http.ResponseWriter, r *http.Request) { + session, ok := m.authenticate(w, r) + if !ok { + return + } + + var body struct { + Folder string `json:"folder"` + UID uint32 `json:"uid"` + } + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&body); err != nil { + writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Invalid JSON") + return + } + if body.Folder == "" { + body.Folder = "inbox" + } + if body.UID == 0 { + writeError(w, http.StatusBadRequest, "INVALID_UID", "Message UID is required") + return + } + + if err := m.useCase.Delete(r.Context(), session.Email, body.Folder, body.UID); err != nil { + m.fail(w, err) + return + } + writeJSON(w, http.StatusOK, map[string]bool{"deleted": 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) @@ -299,6 +330,9 @@ func (m *mailAPI) decodeCompose( // 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"` + // DraftUID is the autosaved draft this message came from, so sending + // can clear it. Without it the draft outlived the message it became. + DraftUID uint32 `json:"draft_uid"` } if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxComposeBytes)).Decode(&body); err != nil { writeError(w, http.StatusBadRequest, "BAD_REQUEST", "Invalid JSON") @@ -314,13 +348,14 @@ func (m *mailAPI) decodeCompose( // letting the body choose From is how a webmail becomes an open relay for // spoofed mail. return usecase.ComposeInput{ - From: sender, - To: body.To, - Cc: body.Cc, - Bcc: body.Bcc, - Subject: body.Subject, - Text: text, - HTML: body.HTML, + From: sender, + To: body.To, + Cc: body.Cc, + Bcc: body.Bcc, + Subject: body.Subject, + Text: text, + HTML: body.HTML, + DraftUID: body.DraftUID, }, true } diff --git a/services/protocols/internal/presentation/http/router.go b/services/protocols/internal/presentation/http/router.go index 929e030..c7bf01d 100644 --- a/services/protocols/internal/presentation/http/router.go +++ b/services/protocols/internal/presentation/http/router.go @@ -89,6 +89,7 @@ func (r *Router) registerRoutes() { 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/mail/delete", r.handleMailDelete) r.mux.HandleFunc("/api/v1/reports/dmarc", r.handleDmarcIngest) r.mux.HandleFunc("/api/v1/reports/tlsrpt", r.handleTlsRptIngest) } @@ -303,6 +304,18 @@ func (r *Router) handleMailSend(w http.ResponseWriter, req *http.Request) { } } +// handleMailDelete removes one message. There was no route for this at all, +// so nothing the webmail displayed could be deleted from it. +func (r *Router) handleMailDelete(w http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPost && req.Method != http.MethodDelete { + writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "POST or DELETE required") + return + } + if r.mailReady(w) { + r.mail.handleDelete(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") diff --git a/services/webmail/next.config.ts b/services/webmail/next.config.ts index 2fade6d..00949be 100644 --- a/services/webmail/next.config.ts +++ b/services/webmail/next.config.ts @@ -16,6 +16,28 @@ const nextConfig: NextConfig = { source: "/api/v1/events", destination: `${PROTOCOLS_SERVICE_URL}/api/v1/events`, }, + // The well-known documents belong to the protocols service, but the + // hostnames senders ask for - mta-sts.