From d33068a43c94bc44544a484d91d87baa6590a394 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Sun, 2 Aug 2026 10:03:03 -0300 Subject: [PATCH] #21: fill in message metadata and stop losing mail to a missing folder Two problems the mail screens exposed, both in the delivery path. The subject, snippet, sender name and message-id columns have existed since the first migration, but nothing ever wrote them: every row went in with those fields NULL. A message list built on the database would have shown blank subjects for every message. Delivery now parses them once per message and stores them, decoding RFC 2047 encoded words so an accented subject is readable rather than raw. Parsing failures are non-fatal: a message with unreadable headers is still delivered, just with less metadata, because refusing it would lose mail over a malformed Subject. Worse, a message the spam filter routed to Junk failed delivery outright when the mailbox had no Junk folder - the sender got 451 and retried forever against a mailbox that would never grow one on its own. Delivery now falls back to INBOX and says so in the log. Found by sending a real message through port 25 against the running stack. The SMTP handler also swallowed the cause of any unexpected failure: the peer saw "temporary error" and the operator got nothing. It is logged now, which is how the missing folder was diagnosed at all. Also adds the JWT verifier the mail API needs. The tokens are minted by the auth service and verified here under the same secret, so the test fixtures are real tokens produced by that service: if the two implementations drift, this fails before the webmail does. Challenge tokens, wrong secrets, tampered payloads, expired tokens and the other surface's audience are all rejected. The dev seed gained the standard folder set, which is what a provisioned mailbox has and what the fallback above should not have to compensate for. Co-Authored-By: Claude Opus 5 --- scripts/dev-seed.sql | 11 ++ .../port/inbound_message_repository.go | 14 ++- .../application/usecase/message_headers.go | 116 ++++++++++++++++++ .../usecase/message_headers_test.go | 88 +++++++++++++ .../usecase/process_inbound_email.go | 9 ++ .../postgres/inbound_message_repository.go | 51 ++++++-- .../inbound_message_repository_test.go | 59 +++++++++ .../internal/presentation/http/websession.go | 102 +++++++++++++++ .../presentation/http/websession_test.go | 93 ++++++++++++++ .../internal/presentation/smtp/backend.go | 5 + 10 files changed, 535 insertions(+), 13 deletions(-) create mode 100644 services/protocols/internal/application/usecase/message_headers.go create mode 100644 services/protocols/internal/application/usecase/message_headers_test.go create mode 100644 services/protocols/internal/presentation/http/websession.go create mode 100644 services/protocols/internal/presentation/http/websession_test.go diff --git a/scripts/dev-seed.sql b/scripts/dev-seed.sql index 1b45afe..0912a58 100644 --- a/scripts/dev-seed.sql +++ b/scripts/dev-seed.sql @@ -28,3 +28,14 @@ VALUES ( NULL ) ON CONFLICT (mailbox_id, name) WHERE (parent_id IS NULL) DO NOTHING; + +-- The standard folder set (RFC 6154). Without Junk in particular, a message +-- the spam filter routes there used to fail delivery outright. +INSERT INTO folders (mailbox_id, name, special_use, parent_id) +SELECT '00000000-0000-0000-0000-000000000002', f.name, f.special_use, NULL + FROM (VALUES ('Sent','sent'), ('Drafts','drafts'), ('Trash','trash'), + ('Junk','junk'), ('Archive','archive')) AS f(name, special_use) + WHERE NOT EXISTS ( + SELECT 1 FROM folders x + WHERE x.mailbox_id = '00000000-0000-0000-0000-000000000002' + AND lower(x.name) = lower(f.name)); diff --git a/services/protocols/internal/application/port/inbound_message_repository.go b/services/protocols/internal/application/port/inbound_message_repository.go index b59e341..eacd107 100644 --- a/services/protocols/internal/application/port/inbound_message_repository.go +++ b/services/protocols/internal/application/port/inbound_message_repository.go @@ -14,9 +14,17 @@ type PersistInboundMessageInput struct { SenderAddress string RecipientAddress string TargetFolderName string // "INBOX", "Junk", etc. (defaults to "INBOX" if empty) - SPFResult string - DKIMResult string - DMARCResult string + // Header-derived fields. The columns for these have existed since the + // first migration but nothing filled them, so every message listed in the + // webmail had a blank subject and sender name. + Subject string + Snippet string + FromDisplayName string + MessageIDHeader string + HasAttachments bool + SPFResult string + DKIMResult string + DMARCResult string } // InboundMessageRepository durably records an accepted inbound message: for diff --git a/services/protocols/internal/application/usecase/message_headers.go b/services/protocols/internal/application/usecase/message_headers.go new file mode 100644 index 0000000..dc1cdd8 --- /dev/null +++ b/services/protocols/internal/application/usecase/message_headers.go @@ -0,0 +1,116 @@ +package usecase + +import ( + "bytes" + "io" + "mime" + "net/mail" + "strings" + "unicode/utf8" +) + +// snippetLength is how much of the body is kept for the message list. It is a +// preview, not the message: the body itself stays in the blob. +const snippetLength = 200 + +// MessageHeaders is the subset of a message the webmail list needs, so that +// showing an inbox does not mean reading every blob off disk. +type MessageHeaders struct { + Subject string + FromDisplayName string + MessageID string + Snippet string + HasAttachments bool +} + +// wordDecoder tolerates the charsets that turn up in real mail. An unknown +// charset yields the raw bytes rather than an error, because a subject that +// renders imperfectly beats a delivery that fails. +var wordDecoder = mime.WordDecoder{ + CharsetReader: func(_ string, input io.Reader) (io.Reader, error) { + return input, nil + }, +} + +func decodeHeader(value string) string { + decoded, err := wordDecoder.DecodeHeader(value) + if err != nil { + return value + } + return decoded +} + +// ExtractMessageHeaders parses the fields the message list displays. +// +// Every failure is non-fatal: a message with unparseable headers is still +// delivered, just with less metadata. Refusing it would lose mail over a +// malformed Subject, which is exactly the kind of thing spam is full of. +func ExtractMessageHeaders(payload []byte) MessageHeaders { + var out MessageHeaders + + msg, err := mail.ReadMessage(bytes.NewReader(payload)) + if err != nil { + return out + } + + out.Subject = truncateUTF8(decodeHeader(msg.Header.Get("Subject")), 998) + out.MessageID = truncateUTF8(strings.Trim(msg.Header.Get("Message-ID"), "<>"), 998) + + if from := msg.Header.Get("From"); from != "" { + if addr, err := mail.ParseAddress(from); err == nil { + out.FromDisplayName = truncateUTF8(decodeHeader(addr.Name), 255) + } else { + // A From that does not parse still often carries a usable name + // before the angle bracket. + if idx := strings.Index(from, "<"); idx > 0 { + out.FromDisplayName = truncateUTF8(decodeHeader(strings.TrimSpace(strings.Trim(from[:idx], `"`))), 255) + } + } + } + + contentType := msg.Header.Get("Content-Type") + mediaType, params, err := mime.ParseMediaType(contentType) + if err == nil && strings.HasPrefix(mediaType, "multipart/") { + // mixed and related carry parts that are not the body; alternative is + // just the same text twice, so it does not count as an attachment. + out.HasAttachments = mediaType != "multipart/alternative" + _ = params + } + + out.Snippet = buildSnippet(msg, mediaType) + return out +} + +// buildSnippet takes the opening of the body as a plain-text preview. +func buildSnippet(msg *mail.Message, mediaType string) string { + // Only a flat text body is previewed. Walking MIME parts here would mean + // decoding attachments on the delivery path, which is the hot path. + if mediaType != "" && !strings.HasPrefix(mediaType, "text/") { + return "" + } + + buf := make([]byte, 8*1024) + n, _ := msg.Body.Read(buf) + if n <= 0 { + return "" + } + + text := string(buf[:n]) + text = strings.ReplaceAll(text, "\r\n", " ") + text = strings.ReplaceAll(text, "\n", " ") + text = strings.Join(strings.Fields(text), " ") + return truncateUTF8(text, snippetLength) +} + +// truncateUTF8 cuts to at most max bytes without splitting a rune, so a +// truncated subject cannot become invalid UTF-8 and break the JSON encoding. +func truncateUTF8(s string, max int) string { + if len(s) <= max { + return s + } + cut := s[:max] + for len(cut) > 0 && !utf8.ValidString(cut) { + cut = cut[:len(cut)-1] + } + return cut +} diff --git a/services/protocols/internal/application/usecase/message_headers_test.go b/services/protocols/internal/application/usecase/message_headers_test.go new file mode 100644 index 0000000..c3c6c76 --- /dev/null +++ b/services/protocols/internal/application/usecase/message_headers_test.go @@ -0,0 +1,88 @@ +package usecase + +import "testing" + +func TestExtractMessageHeaders_ReadsSubjectSenderAndSnippet(t *testing.T) { + msg := "From: Alice Example \r\n" + + "To: bob@example.test\r\n" + + "Subject: Quarterly report\r\n" + + "Message-ID: \r\n" + + "Content-Type: text/plain; charset=utf-8\r\n" + + "\r\n" + + "Here is the report you asked for.\r\nSecond line.\r\n" + + h := ExtractMessageHeaders([]byte(msg)) + + if h.Subject != "Quarterly report" { + t.Errorf("Subject = %q", h.Subject) + } + if h.FromDisplayName != "Alice Example" { + t.Errorf("FromDisplayName = %q", h.FromDisplayName) + } + if h.MessageID != "abc123@remote.test" { + t.Errorf("MessageID = %q, angle brackets should be stripped", h.MessageID) + } + if h.Snippet != "Here is the report you asked for. Second line." { + t.Errorf("Snippet = %q", h.Snippet) + } + if h.HasAttachments { + t.Error("a text/plain message was reported as having attachments") + } +} + +// Real subjects arrive RFC 2047 encoded; showing the raw encoded word in the +// message list is the usual symptom of skipping this. +func TestExtractMessageHeaders_DecodesEncodedWords(t *testing.T) { + msg := "From: =?UTF-8?B?SsO6bGlv?= \r\n" + + "Subject: =?UTF-8?Q?Relat=C3=B3rio_mensal?=\r\n" + + "\r\n" + + "corpo\r\n" + + h := ExtractMessageHeaders([]byte(msg)) + if h.Subject != "Relatório mensal" { + t.Errorf("Subject = %q, want the decoded form", h.Subject) + } + if h.FromDisplayName != "Júlio" { + t.Errorf("FromDisplayName = %q, want the decoded form", h.FromDisplayName) + } +} + +func TestExtractMessageHeaders_FlagsMultipartMixedAsAttachment(t *testing.T) { + withAttachment := "From: a@b.test\r\nSubject: s\r\n" + + "Content-Type: multipart/mixed; boundary=xyz\r\n\r\n--xyz\r\n\r\nbody\r\n--xyz--\r\n" + if !ExtractMessageHeaders([]byte(withAttachment)).HasAttachments { + t.Error("multipart/mixed should be flagged as carrying attachments") + } + + // alternative is the same body twice, not an attachment. + altOnly := "From: a@b.test\r\nSubject: s\r\n" + + "Content-Type: multipart/alternative; boundary=xyz\r\n\r\n--xyz\r\n\r\nbody\r\n--xyz--\r\n" + if ExtractMessageHeaders([]byte(altOnly)).HasAttachments { + t.Error("multipart/alternative should not count as an attachment") + } +} + +// A malformed message must still be delivered - refusing it would lose mail +// over a bad header, which is what spam is full of. +func TestExtractMessageHeaders_SurvivesGarbage(t *testing.T) { + for _, payload := range []string{"", "not a message at all", "Subject: no body follows"} { + h := ExtractMessageHeaders([]byte(payload)) + _ = h // no panic, no error: absent metadata is the worst outcome + } +} + +func TestExtractMessageHeaders_TruncatesWithoutBreakingUTF8(t *testing.T) { + long := "" + for range 300 { + long += "á" + } + h := ExtractMessageHeaders([]byte("Subject: s\r\n\r\n" + long + "\r\n")) + if len(h.Snippet) > snippetLength { + t.Errorf("snippet is %d bytes, want at most %d", len(h.Snippet), snippetLength) + } + for _, r := range h.Snippet { + if r == '�' { + t.Fatal("truncation split a multi-byte rune") + } + } +} diff --git a/services/protocols/internal/application/usecase/process_inbound_email.go b/services/protocols/internal/application/usecase/process_inbound_email.go index 116e04c..63085fc 100644 --- a/services/protocols/internal/application/usecase/process_inbound_email.go +++ b/services/protocols/internal/application/usecase/process_inbound_email.go @@ -234,6 +234,10 @@ func (uc *ProcessInboundEmailUseCase) Handle(ctx context.Context, input ProcessI // would mean a failure halfway through leaves the earlier recipients with // a copy while the sender is told to retry - and the retry delivers to // them a second time. + // Parsed once for the whole delivery: every recipient's row carries the + // same headers, and the list view reads them instead of the blob. + headers := ExtractMessageHeaders(payload) + persistInputs := make([]port.PersistInboundMessageInput, len(input.Recipients)) for i, recipient := range input.Recipients { persistInputs[i] = port.PersistInboundMessageInput{ @@ -242,6 +246,11 @@ func (uc *ProcessInboundEmailUseCase) Handle(ctx context.Context, input ProcessI SenderAddress: input.Sender, RecipientAddress: input.RecipientAddresses[i], TargetFolderName: targetFolder, + Subject: headers.Subject, + Snippet: headers.Snippet, + FromDisplayName: headers.FromDisplayName, + MessageIDHeader: headers.MessageID, + HasAttachments: headers.HasAttachments, SPFResult: authResult.SPF, DKIMResult: authResult.DKIM, DMARCResult: authResult.DMARC, diff --git a/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go b/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go index e7f7905..83c839a 100644 --- a/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go +++ b/services/protocols/internal/infrastructure/postgres/inbound_message_repository.go @@ -2,7 +2,10 @@ package postgres import ( "context" + "errors" "fmt" + "log" + "strings" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" @@ -51,14 +54,10 @@ func (r *InboundMessageRepository) PersistAll(ctx context.Context, inputs []port return uids, nil } -// persistOne writes one recipient's copy inside the caller's transaction. -func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessageInput) (int64, error) { - targetFolder := input.TargetFolderName - if targetFolder == "" { - targetFolder = "INBOX" - } - - var folderID string +// lockFolder resolves a folder by special-use role or name and locks the row, +// which is what serialises UID allocation. +func lockFolder(ctx context.Context, tx pgx.Tx, mailboxID any, folder string) (string, int64, error) { + var id string var uid int64 err := tx.QueryRow(ctx, ` SELECT id, uid_next FROM folders @@ -66,7 +65,36 @@ func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessage ORDER BY special_use IS NOT NULL DESC LIMIT 1 FOR UPDATE - `, input.MailboxID, targetFolder).Scan(&folderID, &uid) + `, mailboxID, folder).Scan(&id, &uid) + return id, uid, err +} + +// nullIfEmpty keeps an absent header as SQL NULL rather than an empty string, +// so "no subject" and "empty subject" stay distinguishable. +func nullIfEmpty(s string) any { + if s == "" { + return nil + } + return s +} + +// persistOne writes one recipient's copy inside the caller's transaction. +func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessageInput) (int64, error) { + targetFolder := input.TargetFolderName + if targetFolder == "" { + targetFolder = "INBOX" + } + + // The target folder is resolved with INBOX as a fallback. A message the + // spam filter routes to Junk must not be refused outright because that + // folder was never created - the delivery is what matters, and refusing it + // makes the sender retry forever against a mailbox that will never grow a + // Junk folder on its own. + folderID, uid, err := lockFolder(ctx, tx, input.MailboxID, targetFolder) + if errors.Is(err, pgx.ErrNoRows) && !strings.EqualFold(targetFolder, "INBOX") { + log.Printf("delivery: mailbox %s has no %q folder, filing in INBOX instead", input.MailboxID, targetFolder) + folderID, uid, err = lockFolder(ctx, tx, input.MailboxID, "INBOX") + } if err != nil { return 0, fmt.Errorf("find folder %q for mailbox %s: %w", targetFolder, input.MailboxID, err) } @@ -85,10 +113,13 @@ func persistOne(ctx context.Context, tx pgx.Tx, input port.PersistInboundMessage 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) + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) `, 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), + nullIfEmpty(input.MessageIDHeader), input.HasAttachments, input.SPFResult, input.DKIMResult, input.DMARCResult) if err != nil { return 0, fmt.Errorf("insert email_messages: %w", err) diff --git a/services/protocols/internal/infrastructure/postgres/inbound_message_repository_test.go b/services/protocols/internal/infrastructure/postgres/inbound_message_repository_test.go index 500f296..d1b40e6 100644 --- a/services/protocols/internal/infrastructure/postgres/inbound_message_repository_test.go +++ b/services/protocols/internal/infrastructure/postgres/inbound_message_repository_test.go @@ -204,3 +204,62 @@ func TestInboundMessageRepository_PersistAll_RollsBackEveryRecipientOnFailure(t t.Errorf("ref_count = %d after a rolled-back delivery, want 0", refCount) } } + +// A message the spam filter routes to Junk must still be delivered when the +// mailbox has no Junk folder. Refusing it made the sender retry forever +// against a mailbox that would never grow one on its own. +func TestInboundMessageRepository_FallsBackToInboxWhenFolderMissing(t *testing.T) { + pool := testPool(t) + defer pool.Close() + ctx := context.Background() + + domainID := uuid.New() + domainName := "fallback-test-" + domainID.String() + ".invalid" + if _, err := pool.Exec(ctx, `INSERT INTO domains (id, name, punycode_name) VALUES ($1, $2, $3)`, + domainID, domainName, domainName); err != nil { + t.Fatalf("seed domain: %v", err) + } + defer pool.Exec(ctx, `DELETE FROM domains WHERE id = $1`, domainID) + + mailboxID := uuid.New() + if _, err := pool.Exec(ctx, `INSERT INTO mailboxes (id, domain_id, local_part, email_address, password_hash) + VALUES ($1, $2, 'nojunk', $3, 'x')`, mailboxID, domainID, "nojunk@"+domainName); err != nil { + t.Fatalf("seed mailbox: %v", err) + } + + // Only an INBOX exists - deliberately no Junk folder. + inboxID := uuid.New() + if _, err := pool.Exec(ctx, `INSERT INTO folders (id, mailbox_id, name, special_use) VALUES ($1, $2, 'INBOX', 'inbox')`, + inboxID, mailboxID); err != nil { + t.Fatalf("seed folder: %v", err) + } + + blobID := uuid.New() + if _, err := pool.Exec(ctx, `INSERT INTO message_blobs (id, content_sha256, storage_driver, storage_path, size_bytes) VALUES ($1, $2, 'local', '/tmp/x', 10)`, + blobID, testBlobDigest()); err != nil { + t.Fatalf("seed blob: %v", err) + } + defer pool.Exec(ctx, `DELETE FROM message_blobs WHERE id = $1`, blobID) + + repo := NewInboundMessageRepository(pool) + uids, err := repo.PersistAll(ctx, []port.PersistInboundMessageInput{{ + MailboxID: mailboxID, Blob: port.BlobRef{ID: blobID, SizeBytes: 10}, + SenderAddress: "spammer@remote.test", RecipientAddress: "nojunk@" + domainName, + TargetFolderName: "Junk", + SPFResult: "none", DKIMResult: "none", DMARCResult: "none", + }}) + if err != nil { + t.Fatalf("delivery to a missing folder failed instead of falling back: %v", err) + } + if len(uids) != 1 { + t.Fatalf("got %d uids, want 1", len(uids)) + } + + var folderID uuid.UUID + if err := pool.QueryRow(ctx, `SELECT folder_id FROM email_messages WHERE mailbox_id = $1`, mailboxID).Scan(&folderID); err != nil { + t.Fatalf("query message: %v", err) + } + if folderID != inboxID { + t.Errorf("message landed in folder %s, want the INBOX %s", folderID, inboxID) + } +} diff --git a/services/protocols/internal/presentation/http/websession.go b/services/protocols/internal/presentation/http/websession.go new file mode 100644 index 0000000..225a4ac --- /dev/null +++ b/services/protocols/internal/presentation/http/websession.go @@ -0,0 +1,102 @@ +package httppresentation + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "strings" + "time" +) + +// WebSession is the claim set the auth service puts in its session tokens. +// The two services share only this shape and the signing secret; the tokens +// themselves are issued in one place and verified in both. +type WebSession struct { + Subject string `json:"sub"` + Email string `json:"email"` + Role string `json:"role"` + DomainID string `json:"domainId"` + Surface string `json:"surface"` + Audience string `json:"aud"` + MfaSatisfied bool `json:"mfaSatisfied"` + Purpose string `json:"purpose"` + IssuedAt int64 `json:"iat"` + ExpiresAt int64 `json:"exp"` +} + +var ( + ErrTokenMalformed = errors.New("web session: token is not a three-part JWT") + ErrBadSignature = errors.New("web session: signature does not verify") + ErrTokenExpired = errors.New("web session: token has expired") + ErrNotASession = errors.New("web session: token is not a session token") + ErrWrongAudience = errors.New("web session: token was issued for a different surface") +) + +// WebSessionVerifier validates the tokens minted by the auth service. +// +// The secret is shared rather than the tokens being re-checked over HTTP: a +// round trip to the auth service on every mail request would put a second +// network hop in the read path for no extra safety, since both services +// already trust the same secret to say who the user is. +type WebSessionVerifier struct { + secret []byte + now func() time.Time +} + +func NewWebSessionVerifier(secret string) *WebSessionVerifier { + return &WebSessionVerifier{secret: []byte(secret), now: time.Now} +} + +// Verify checks the signature, the expiry, and that this is a real session +// rather than the short-lived challenge issued between the password and the +// second factor. +func (v *WebSessionVerifier) Verify(token string) (*WebSession, error) { + parts := strings.Split(token, ".") + if len(parts) != 3 { + return nil, ErrTokenMalformed + } + + mac := hmac.New(sha256.New, v.secret) + mac.Write([]byte(parts[0] + "." + parts[1])) + expected := base64.RawURLEncoding.EncodeToString(mac.Sum(nil)) + + // Compared with hmac.Equal so a wrong signature costs the same time as a + // right one, whatever the mismatch position. + if !hmac.Equal([]byte(parts[2]), []byte(expected)) { + return nil, ErrBadSignature + } + + raw, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return nil, ErrTokenMalformed + } + + var session WebSession + if err := json.Unmarshal(raw, &session); err != nil { + return nil, ErrTokenMalformed + } + + if session.ExpiresAt > 0 && v.now().Unix() >= session.ExpiresAt { + return nil, ErrTokenExpired + } + if session.Purpose != "session" { + return nil, ErrNotASession + } + + return &session, nil +} + +// RequireSurface additionally pins the token to one surface, which is what +// keeps a /user token out of an admin-only endpoint (PLAN.md section 14.1). +func (v *WebSessionVerifier) RequireSurface(token, surface string) (*WebSession, error) { + session, err := v.Verify(token) + if err != nil { + return nil, err + } + if session.Surface != surface || session.Audience != "lambdamail:"+surface { + return nil, ErrWrongAudience + } + return session, nil +} diff --git a/services/protocols/internal/presentation/http/websession_test.go b/services/protocols/internal/presentation/http/websession_test.go new file mode 100644 index 0000000..4848eb5 --- /dev/null +++ b/services/protocols/internal/presentation/http/websession_test.go @@ -0,0 +1,93 @@ +package httppresentation + +import ( + "errors" + "testing" + "time" +) + +// These tokens were minted by the auth service itself (services/auth, +// session.ts) under the secret below. They are the real wire format: if the +// two implementations ever drift, every mail request from the webmail starts +// failing and this test says so first. +const ( + interopSecret = "shared-secret-for-interop-test" + interopSession = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJtYi0xIiwiZW1haWwiOiJ1c2VyQGV4YW1wbGUudGVzdCIsInJvbGUiOiJVU0VSIiwiZG9tYWluSWQiOiJkb20tMSIsInN1cmZhY2UiOiJ1c2VyIiwiYXVkIjoibGFtYmRhbWFpbDp1c2VyIiwibWZhU2F0aXNmaWVkIjp0cnVlLCJtZmFTYXRpc2ZpZWRBdCI6MTc4NTY3NDI0MzYzMiwicHVycG9zZSI6InNlc3Npb24iLCJpYXQiOjE3ODU2NzQyNDMsImV4cCI6MTc4NTcwMzA0M30.vrXM2T9HKfHDOThmx0LD_n05jfsr4iaKYpZrKTBa5NM" + interopChallenge = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJtYi0xIiwiZW1haWwiOiJ1c2VyQGV4YW1wbGUudGVzdCIsInJvbGUiOiJVU0VSIiwiZG9tYWluSWQiOiJkb20tMSIsInN1cmZhY2UiOiJ1c2VyIiwiYXVkIjoibGFtYmRhbWFpbDp1c2VyIiwibWZhU2F0aXNmaWVkIjpmYWxzZSwicHVycG9zZSI6Im1mYV9jaGFsbGVuZ2UiLCJpYXQiOjE3ODU2NzQyNDMsImV4cCI6MTc4NTY3NDU0M30.7OqmI9A1L9Efbt6AX4lqojZZ8IKuWi-6oe_VJNgJAxM" + // The captured tokens carry a fixed validity window. Verification is + // pinned to an instant inside it, so these tests assert on the signature + // and the claims rather than quietly turning red once the tokens age out. + interopIssuedAt = 1785674243 +) + +func testVerifier(secret string) *WebSessionVerifier { + v := NewWebSessionVerifier(secret) + v.now = func() time.Time { return time.Unix(interopIssuedAt+60, 0) } + return v +} + +func TestWebSessionVerifier_AcceptsTokenIssuedByTheAuthService(t *testing.T) { + session, err := testVerifier(interopSecret).Verify(interopSession) + if err != nil { + t.Fatalf("a token minted by the auth service was rejected: %v", err) + } + if session.Email != "user@example.test" { + t.Errorf("Email = %q, want user@example.test", session.Email) + } + if session.Surface != "user" || session.Audience != "lambdamail:user" { + t.Errorf("surface/aud = %q/%q, want user/lambdamail:user", session.Surface, session.Audience) + } +} + +// The challenge token is issued after the password but before the second +// factor. Accepting it here would let the mail API be opened without MFA. +func TestWebSessionVerifier_RejectsChallengeToken(t *testing.T) { + _, err := testVerifier(interopSecret).Verify(interopChallenge) + if !errors.Is(err, ErrNotASession) { + t.Fatalf("error = %v, want ErrNotASession", err) + } +} + +func TestWebSessionVerifier_RejectsWrongSecret(t *testing.T) { + _, err := testVerifier("not-the-secret").Verify(interopSession) + if !errors.Is(err, ErrBadSignature) { + t.Fatalf("error = %v, want ErrBadSignature", err) + } +} + +// A tampered payload must not survive: this is the check that stops a user +// from promoting their own token to the admin surface. +func TestWebSessionVerifier_RejectsTamperedPayload(t *testing.T) { + tampered := interopSession[:len(interopSession)-6] + "AAAAAA" + if _, err := testVerifier(interopSecret).Verify(tampered); err == nil { + t.Fatal("a tampered token verified") + } +} + +func TestWebSessionVerifier_RequireSurfaceRejectsOtherSurface(t *testing.T) { + _, err := testVerifier(interopSecret).RequireSurface(interopSession, "admin") + if !errors.Is(err, ErrWrongAudience) { + t.Fatalf("error = %v, want ErrWrongAudience", err) + } + if _, err := testVerifier(interopSecret).RequireSurface(interopSession, "user"); err != nil { + t.Fatalf("the matching surface was rejected: %v", err) + } +} + +func TestWebSessionVerifier_RejectsMalformed(t *testing.T) { + v := testVerifier(interopSecret) + for _, token := range []string{"", "a.b", "a.b.c.d", "not-a-token"} { + if _, err := v.Verify(token); err == nil { + t.Errorf("token %q verified", token) + } + } +} + +// Expiry still has to bite: pinning the clock must not disable the check. +func TestWebSessionVerifier_RejectsExpiredToken(t *testing.T) { + v := NewWebSessionVerifier(interopSecret) + v.now = func() time.Time { return time.Unix(interopIssuedAt+9*60*60, 0) } + if _, err := v.Verify(interopSession); !errors.Is(err, ErrTokenExpired) { + t.Fatalf("error = %v, want ErrTokenExpired", err) + } +} diff --git a/services/protocols/internal/presentation/smtp/backend.go b/services/protocols/internal/presentation/smtp/backend.go index 255a5d1..f4b588a 100644 --- a/services/protocols/internal/presentation/smtp/backend.go +++ b/services/protocols/internal/presentation/smtp/backend.go @@ -7,6 +7,7 @@ import ( "context" "errors" "io" + "log" "net" gosmtp "github.com/emersion/go-smtp" @@ -157,6 +158,10 @@ func (s *session) Data(r io.Reader) error { // Anything else is an internal failure. It is reported as temporary so // the sender retries: the message was not stored, and telling the peer // it failed permanently would discard mail over a bug on this side. + // + // Logged because the peer only ever sees "temporary error"; without + // this line an operator has a bounced delivery and no cause. + log.Printf("smtp: could not store message from %s: %v", s.from, err) return &gosmtp.SMTPError{ Code: 451, EnhancedCode: gosmtp.EnhancedCode{4, 3, 0},