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},