diff --git a/internal/domain/email_provider.go b/internal/domain/email_provider.go index ec46fb4f..5e5f5d25 100644 --- a/internal/domain/email_provider.go +++ b/internal/domain/email_provider.go @@ -372,6 +372,11 @@ type SendEmailProviderRequest struct { Provider *EmailProvider `validate:"required"` EmailOptions EmailOptions + // PlainText is the optional text/plain alternative body. Empty for templates that + // don't set one (EmailTemplate.Text is nullable) — providers must send HTML-only + // in that case rather than a text/plain part with empty content. + PlainText string + // CapturedMessageID, when non-nil, is written by providers that OVERWRITE the RFC // Message-ID at send time (e.g. Amazon SES) with the provider-returned MessageId, so // the caller can store the recipient-visible Message-ID for reply matching. It is a diff --git a/internal/service/email_service.go b/internal/service/email_service.go index 820b5876..6d5af220 100644 --- a/internal/service/email_service.go +++ b/internal/service/email_service.go @@ -598,6 +598,29 @@ func (s *EmailService) SendEmailForTemplate(ctx context.Context, request domain. htmlContent := *compiledTemplate.HTML + // Plain-text alternative: same Liquid variables as the subject, but unlike the + // subject a rendering failure here must not fail the send — it's a deliverability + // enhancement (multipart/alternative), not the message itself. Fall back to the + // raw, unrendered text so recipients still get readable content instead of losing + // the alternative part entirely. + var plainTextContent string + if emailContent.Text != nil && *emailContent.Text != "" { + processedText, err := notifuse_mjml.ProcessLiquidTemplate( + *emailContent.Text, + request.MessageData.Data, + "email_plain_text", + ) + if err != nil { + s.logger.WithFields(map[string]interface{}{ + "error": err.Error(), + "message_id": request.MessageID, + "template_id": request.TemplateConfig.TemplateID, + }).Warn("Failed to process plain-text alternative with Liquid templating, sending it unrendered") + processedText = *emailContent.Text + } + plainTextContent = processedText + } + now := time.Now().UTC() // Convert email options to channel options for storage @@ -657,6 +680,7 @@ func (s *EmailService) SendEmailForTemplate(ctx context.Context, request domain. To: request.Contact.Email, Subject: subject, Content: htmlContent, + PlainText: plainTextContent, Provider: request.EmailProvider, EmailOptions: request.EmailOptions, } diff --git a/internal/service/email_service_test.go b/internal/service/email_service_test.go index fbcdc315..f106ccde 100644 --- a/internal/service/email_service_test.go +++ b/internal/service/email_service_test.go @@ -1075,6 +1075,7 @@ func TestEmailService_SendEmailForTemplate(t *testing.T) { mockLogger.EXPECT().WithField(gomock.Any(), gomock.Any()).Return(mockLogger).AnyTimes() mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() mockLogger.EXPECT().Info(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Warn(gomock.Any()).AnyTimes() mockLogger.EXPECT().Error(gomock.Any()).AnyTimes() // Create email template @@ -1168,6 +1169,152 @@ func TestEmailService_SendEmailForTemplate(t *testing.T) { require.NoError(t, err) }) + t.Run("plain-text alternative is rendered through Liquid and passed to the provider", func(t *testing.T) { + plainText := "Hello {{name}}, visit {{link}}" + emailTemplate.Email.Text = &plainText + defer func() { emailTemplate.Email.Text = nil }() + + workspace := &domain.Workspace{ + ID: workspaceID, + Settings: domain.WorkspaceSettings{}, + } + mockWorkspaceRepo.EXPECT(). + GetByID(gomock.Any(), workspaceID). + Return(workspace, nil) + mockTemplateService.EXPECT(). + GetTemplateByID(gomock.Any(), workspaceID, templateConfig.TemplateID, int64(0)). + Return(emailTemplate, nil) + mockTemplateService.EXPECT(). + CompileTemplate(gomock.Any(), gomock.Any()). + Return(compileResult, nil) + mockMessageRepo.EXPECT(). + Create(gomock.Any(), workspaceID, gomock.Any(), gomock.Any()). + Return(nil) + + var capturedRequest domain.SendEmailProviderRequest + mockSESService.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, req domain.SendEmailProviderRequest) error { + capturedRequest = req + return nil + }) + + request := domain.SendEmailRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: messageID, + ExternalID: nil, + Contact: contact, + TemplateConfig: templateConfig, + MessageData: messageData, + TrackingSettings: trackingSettings, + EmailProvider: emailProvider, + EmailOptions: options, + } + err := emailService.SendEmailForTemplate(ctx, request) + + require.NoError(t, err) + assert.Equal(t, "Hello Test User, visit https://example.com/test", capturedRequest.PlainText) + }) + + t.Run("plain-text alternative is left empty when the template has none", func(t *testing.T) { + workspace := &domain.Workspace{ + ID: workspaceID, + Settings: domain.WorkspaceSettings{}, + } + mockWorkspaceRepo.EXPECT(). + GetByID(gomock.Any(), workspaceID). + Return(workspace, nil) + mockTemplateService.EXPECT(). + GetTemplateByID(gomock.Any(), workspaceID, templateConfig.TemplateID, int64(0)). + Return(emailTemplate, nil) + mockTemplateService.EXPECT(). + CompileTemplate(gomock.Any(), gomock.Any()). + Return(compileResult, nil) + mockMessageRepo.EXPECT(). + Create(gomock.Any(), workspaceID, gomock.Any(), gomock.Any()). + Return(nil) + + var capturedRequest domain.SendEmailProviderRequest + mockSESService.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, req domain.SendEmailProviderRequest) error { + capturedRequest = req + return nil + }) + + request := domain.SendEmailRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: messageID, + ExternalID: nil, + Contact: contact, + TemplateConfig: templateConfig, + MessageData: messageData, + TrackingSettings: trackingSettings, + EmailProvider: emailProvider, + EmailOptions: options, + } + err := emailService.SendEmailForTemplate(ctx, request) + + require.NoError(t, err) + assert.Empty(t, capturedRequest.PlainText) + }) + + t.Run("plain-text alternative Liquid error falls back to the raw text instead of failing the send", func(t *testing.T) { + // Oversized Liquid content is a reliable, deterministic way to make + // processLiquidContent error (see SecureLiquidEngine's size limit in + // pkg/notifuse_mjml/liquid_secure_test.go) without depending on undocumented + // parser leniency for malformed syntax. + plainText := "{{name}} " + strings.Repeat("x", 200000) + emailTemplate.Email.Text = &plainText + defer func() { emailTemplate.Email.Text = nil }() + + workspace := &domain.Workspace{ + ID: workspaceID, + Settings: domain.WorkspaceSettings{}, + } + mockWorkspaceRepo.EXPECT(). + GetByID(gomock.Any(), workspaceID). + Return(workspace, nil) + mockTemplateService.EXPECT(). + GetTemplateByID(gomock.Any(), workspaceID, templateConfig.TemplateID, int64(0)). + Return(emailTemplate, nil) + mockTemplateService.EXPECT(). + CompileTemplate(gomock.Any(), gomock.Any()). + Return(compileResult, nil) + mockMessageRepo.EXPECT(). + Create(gomock.Any(), workspaceID, gomock.Any(), gomock.Any()). + Return(nil) + + var capturedRequest domain.SendEmailProviderRequest + mockSESService.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, req domain.SendEmailProviderRequest) error { + capturedRequest = req + return nil + }) + + request := domain.SendEmailRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: messageID, + ExternalID: nil, + Contact: contact, + TemplateConfig: templateConfig, + MessageData: messageData, + TrackingSettings: trackingSettings, + EmailProvider: emailProvider, + EmailOptions: options, + } + err := emailService.SendEmailForTemplate(ctx, request) + + // The send must still succeed — an unrenderable plain-text alternative is a + // deliverability nice-to-have, not a reason to drop the message. + require.NoError(t, err) + assert.Equal(t, plainText, capturedRequest.PlainText) + }) + t.Run("TrackingMode survives the compile-request rebuild", func(t *testing.T) { workspace := &domain.Workspace{ ID: workspaceID, diff --git a/internal/service/mailgun_service.go b/internal/service/mailgun_service.go index 6305a572..68882bb1 100644 --- a/internal/service/mailgun_service.go +++ b/internal/service/mailgun_service.go @@ -780,6 +780,9 @@ func (s *MailgunService) sendEmailSimple(ctx context.Context, apiURL string, req form.Add("to", request.To) form.Add("subject", request.Subject) form.Add("html", request.Content) + if request.PlainText != "" { + form.Add("text", request.PlainText) + } // Add cc recipients if provided for _, ccAddress := range request.EmailOptions.CC { @@ -860,6 +863,11 @@ func (s *MailgunService) sendEmailWithAttachments(ctx context.Context, apiURL st if err := writer.WriteField("html", request.Content); err != nil { return fmt.Errorf("failed to write html field: %w", err) } + if request.PlainText != "" { + if err := writer.WriteField("text", request.PlainText); err != nil { + return fmt.Errorf("failed to write text field: %w", err) + } + } // Add cc recipients if provided for _, ccAddress := range request.EmailOptions.CC { diff --git a/internal/service/mailgun_service_test.go b/internal/service/mailgun_service_test.go index 58df6aa5..ce1bdf6d 100644 --- a/internal/service/mailgun_service_test.go +++ b/internal/service/mailgun_service_test.go @@ -782,6 +782,54 @@ func TestMailgunService_SendEmail(t *testing.T) { require.NoError(t, err) }) + t.Run("with plain text alternative", func(t *testing.T) { + ctx := context.Background() + plainText := "Test Email Content" + + provider := &domain.EmailProvider{ + Mailgun: &domain.MailgunSettings{ + Domain: "example.com", + APIKey: "test-api-key", + Region: "US", + }, + } + + resp := &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"id": "", "message": "Queued. Thank you."}`)), + } + + mockHTTPClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + formData := string(body) + + assert.Contains(t, formData, "html="+url.QueryEscape(content)) + assert.Contains(t, formData, "text="+url.QueryEscape(plainText)) + + return resp, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: fromAddress, + FromName: fromName, + To: to, + Subject: subject, + Content: content, + PlainText: plainText, + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + err := service.SendEmail(ctx, request) + + require.NoError(t, err) + }) + t.Run("EU region", func(t *testing.T) { ctx := context.Background() @@ -1034,6 +1082,70 @@ func TestMailgunService_SendEmail(t *testing.T) { require.NoError(t, err) }) + t.Run("with plain text alternative and attachment", func(t *testing.T) { + ctx := context.Background() + plainText := "Test Email Content" + + provider := &domain.EmailProvider{ + Mailgun: &domain.MailgunSettings{ + Domain: "example.com", + APIKey: "test-api-key", + Region: "US", + }, + } + + base64Content := "c2FtcGxlIHBkZiBjb250ZW50" // base64 of "sample pdf content" + + resp := &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"id": "", "message": "Queued. Thank you."}`)), + } + + mockLogger.EXPECT().WithField(gomock.Any(), gomock.Any()).Return(mockLogger).AnyTimes() + mockLogger.EXPECT().Debug(gomock.Any()).AnyTimes() + + mockHTTPClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + bodyStr := string(body) + + assert.Contains(t, bodyStr, `name="html"`) + assert.Contains(t, bodyStr, `name="text"`) + assert.Contains(t, bodyStr, plainText) + assert.Contains(t, bodyStr, "filename=\"invoice.pdf\"") + + return resp, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: fromAddress, + FromName: fromName, + To: to, + Subject: subject, + Content: content, + PlainText: plainText, + Provider: provider, + EmailOptions: domain.EmailOptions{ + Attachments: []domain.Attachment{ + { + Filename: "invoice.pdf", + Content: base64Content, + ContentType: "application/pdf", + Disposition: "attachment", + }, + }, + }, + } + err := service.SendEmail(ctx, request) + + require.NoError(t, err) + }) + t.Run("email with multiple attachments", func(t *testing.T) { ctx := context.Background() diff --git a/internal/service/mailjet_service.go b/internal/service/mailjet_service.go index cff70259..7bd525d6 100644 --- a/internal/service/mailjet_service.go +++ b/internal/service/mailjet_service.go @@ -534,6 +534,7 @@ func (s *MailjetService) SendEmail(ctx context.Context, request domain.SendEmail }, Subject: request.Subject, HTMLPart: request.Content, + TextPart: request.PlainText, CustomID: request.MessageID, } diff --git a/internal/service/mailjet_service_test.go b/internal/service/mailjet_service_test.go index 391a0750..aebdafb1 100644 --- a/internal/service/mailjet_service_test.go +++ b/internal/service/mailjet_service_test.go @@ -516,6 +516,59 @@ func TestMailjetService_SendEmail(t *testing.T) { require.NoError(t, err) }) + t.Run("Successfully send email with plain text alternative", func(t *testing.T) { + ctx := context.Background() + plainText := "Test Email Content" + + provider := &domain.EmailProvider{ + Mailjet: &domain.MailjetSettings{ + APIKey: "test-api-key", + SecretKey: "test-secret-key", + }, + } + + expectedResponse := map[string]interface{}{ + "Messages": []map[string]interface{}{ + {"Status": "success"}, + }, + } + + mockHTTPClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + + var emailReq map[string]interface{} + err = json.Unmarshal(body, &emailReq) + require.NoError(t, err) + + messages := emailReq["Messages"].([]interface{}) + message := messages[0].(map[string]interface{}) + assert.Equal(t, content, message["HTMLPart"]) + assert.Equal(t, plainText, message["TextPart"]) + + return mockHTTPResponse(t, http.StatusOK, expectedResponse), nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: messageID, + FromAddress: fromAddress, + FromName: fromName, + To: to, + Subject: subject, + Content: content, + PlainText: plainText, + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + err := service.SendEmail(ctx, request) + + require.NoError(t, err) + }) + t.Run("Missing Mailjet configuration", func(t *testing.T) { ctx := context.Background() diff --git a/internal/service/postmark_service.go b/internal/service/postmark_service.go index 61cefad6..614624ae 100644 --- a/internal/service/postmark_service.go +++ b/internal/service/postmark_service.go @@ -556,6 +556,10 @@ func (s *PostmarkService) SendEmail(ctx context.Context, request domain.SendEmai }, } + if request.PlainText != "" { + requestBody["TextBody"] = request.PlainText + } + // Add CC if specified if len(request.EmailOptions.CC) > 0 { var ccAddresses []string diff --git a/internal/service/postmark_service_test.go b/internal/service/postmark_service_test.go index a9dbe352..96f1a616 100644 --- a/internal/service/postmark_service_test.go +++ b/internal/service/postmark_service_test.go @@ -1612,6 +1612,51 @@ func TestPostmarkService_SendEmail(t *testing.T) { assert.NoError(t, err) }) + t.Run("Successfully send email with plain text alternative", func(t *testing.T) { + // Setup + service, httpClient, _, _ := setupPostmarkTest(t) + workspaceID := "workspace-123" + content := "

This is a test email

" + plainText := "This is a test email" + + providerConfig := &domain.EmailProvider{ + Kind: domain.EmailProviderKindPostmark, + Postmark: &domain.PostmarkSettings{ + ServerToken: "test-server-token", + }, + } + + httpClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, _ := io.ReadAll(req.Body) + var requestBody map[string]interface{} + err := json.Unmarshal(body, &requestBody) + require.NoError(t, err) + assert.Equal(t, content, requestBody["HtmlBody"]) + assert.Equal(t, plainText, requestBody["TextBody"]) + + return createMockResponse(http.StatusOK, `{"MessageID":"12345"}`), nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: "sender@example.com", + FromName: "Sender Name", + To: "recipient@example.com", + Subject: "Test Email", + Content: content, + PlainText: plainText, + Provider: providerConfig, + EmailOptions: domain.EmailOptions{}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) + }) + t.Run("Missing Postmark configuration", func(t *testing.T) { // Setup service, _, _, _ := setupPostmarkTest(t) diff --git a/internal/service/sendgrid_service.go b/internal/service/sendgrid_service.go index 9d46bd3e..eec5d8e6 100644 --- a/internal/service/sendgrid_service.go +++ b/internal/service/sendgrid_service.go @@ -320,6 +320,14 @@ func (s *SendGridService) SendEmail(ctx context.Context, request domain.SendEmai } } + // SendGrid requires text/plain to precede text/html when both are present. + // https://www.twilio.com/docs/sendgrid/api-reference/mail-send/mail-send#body + content := []Content{} + if request.PlainText != "" { + content = append(content, Content{Type: "text/plain", Value: request.PlainText}) + } + content = append(content, Content{Type: "text/html", Value: request.Content}) + // Build the mail request mailReq := MailSendRequest{ Personalizations: []Personalization{personalization}, @@ -328,12 +336,7 @@ func (s *SendGridService) SendEmail(ctx context.Context, request domain.SendEmai Name: request.FromName, }, Subject: request.Subject, - Content: []Content{ - { - Type: "text/html", - Value: request.Content, - }, - }, + Content: content, CustomArgs: map[string]string{ "notifuse_message_id": request.MessageID, }, diff --git a/internal/service/sendgrid_service_test.go b/internal/service/sendgrid_service_test.go index ceb21611..35b7fde4 100644 --- a/internal/service/sendgrid_service_test.go +++ b/internal/service/sendgrid_service_test.go @@ -11,6 +11,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/Notifuse/notifuse/internal/domain" "github.com/Notifuse/notifuse/internal/domain/mocks" @@ -440,6 +441,55 @@ func TestSendGridService_SendEmail(t *testing.T) { assert.NoError(t, err) }) + t.Run("Success with plain text alternative", func(t *testing.T) { + ctx := context.Background() + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace-123", + IntegrationID: "integration-456", + MessageID: "msg-789", + FromAddress: "sender@example.com", + FromName: "Sender Name", + To: "recipient@example.com", + Subject: "Test Subject", + Content: "

Test content

", + PlainText: "Test content", + Provider: &domain.EmailProvider{ + Kind: domain.EmailProviderKindSendGrid, + SendGrid: &domain.SendGridSettings{ + APIKey: "SG.test-api-key", + }, + }, + } + + mockHTTPClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, _ := io.ReadAll(req.Body) + + var parsed struct { + Content []struct { + Type string `json:"type"` + Value string `json:"value"` + } `json:"content"` + } + require.NoError(t, json.Unmarshal(body, &parsed)) + + // text/plain must precede text/html per SendGrid's requirement. + require.Len(t, parsed.Content, 2) + assert.Equal(t, "text/plain", parsed.Content[0].Type) + assert.Equal(t, "Test content", parsed.Content[0].Value) + assert.Equal(t, "text/html", parsed.Content[1].Type) + assert.Equal(t, "

Test content

", parsed.Content[1].Value) + + return mockSendGridHTTPResponse(http.StatusAccepted, `{}`), nil + }) + + err := sendGridService.SendEmail(ctx, request) + + assert.NoError(t, err) + }) + t.Run("Success with email options", func(t *testing.T) { ctx := context.Background() diff --git a/internal/service/ses_service.go b/internal/service/ses_service.go index 83d9273f..673cb318 100644 --- a/internal/service/ses_service.go +++ b/internal/service/ses_service.go @@ -19,17 +19,17 @@ import ( "github.com/Notifuse/notifuse/internal/domain" "github.com/Notifuse/notifuse/pkg/logger" + awsv2 "github.com/aws/aws-sdk-go-v2/aws" + awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http" + credentialsv2 "github.com/aws/aws-sdk-go-v2/credentials" + "github.com/aws/aws-sdk-go-v2/service/sesv2" + sesv2types "github.com/aws/aws-sdk-go-v2/service/sesv2/types" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ses" "github.com/aws/aws-sdk-go/service/sns" - awsv2 "github.com/aws/aws-sdk-go-v2/aws" - awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http" - credentialsv2 "github.com/aws/aws-sdk-go-v2/credentials" - "github.com/aws/aws-sdk-go-v2/service/sesv2" - sesv2types "github.com/aws/aws-sdk-go-v2/service/sesv2/types" "github.com/aws/smithy-go" "golang.org/x/net/idna" "golang.org/x/sync/singleflight" @@ -200,9 +200,9 @@ func NewSESServiceWithClients( // connections are pooled across sends. func newSESv2Client(config domain.AmazonSESSettings) domain.SESv2Client { return sesv2.NewFromConfig(awsv2.Config{ - Region: config.Region, - Credentials: credentialsv2.NewStaticCredentialsProvider(config.AccessKey, config.SecretKey, ""), - HTTPClient: sharedSESHTTPClient, + Region: config.Region, + Credentials: credentialsv2.NewStaticCredentialsProvider(config.AccessKey, config.SecretKey, ""), + HTTPClient: sharedSESHTTPClient, // v1 defaulted to 3 RETRIES (aws/client/default_retryer.go:40), i.e. 4 attempts, while // v2 counts total ATTEMPTS. Using 4 keeps the send path exactly as resilient to SES // throttling as it was before the migration. @@ -665,9 +665,9 @@ func (s *SESService) RegisterWebhooks( "integration_id": integrationID, "workspace_id": workspaceID, "aws_region": providerConfig.SES.Region, - "delivery_topic": topicARN, - "bounce_topic": topicARN, - "complaint_topic": topicARN, + "delivery_topic": topicARN, + "bounce_topic": topicARN, + "complaint_topic": topicARN, }, } @@ -1365,6 +1365,7 @@ func (s *SESService) SendEmail(ctx context.Context, request domain.SendEmailProv Charset: awsv2.String("UTF-8"), Data: awsv2.String(request.Content), }, + Text: buildSESTextContent(request.PlainText), }, Subject: &sesv2types.Content{ Charset: awsv2.String("UTF-8"), @@ -1427,6 +1428,18 @@ func buildSESDestination(encodedTo string, cc, bcc []string) (*sesv2types.Destin return destination, nil } +// buildSESTextContent returns the plain-text Body part, or nil when there is none — SES rejects +// an empty Data string the same way it rejects empty configuration-set names. +func buildSESTextContent(plainText string) *sesv2types.Content { + if plainText == "" { + return nil + } + return &sesv2types.Content{ + Charset: awsv2.String("UTF-8"), + Data: awsv2.String(plainText), + } +} + // applySESSendingContext attaches the configuration set, the tenant and the message-id tag. // Empty names stay nil rather than becoming empty strings, which SES rejects. func applySESSendingContext(input *sesv2.SendEmailInput, configSetName, tenantName, messageID string) { @@ -1537,6 +1550,28 @@ func (s *SESService) sendRawEmail(ctx context.Context, sesClient domain.SESv2Cli return nil } + // writeTextPart writes the quoted-printable plain-text body into the given writer. + writeTextPart := func(w *multipart.Writer) error { + textPart := textproto.MIMEHeader{} + textPart.Set("Content-Type", "text/plain; charset=UTF-8") + textPart.Set("Content-Transfer-Encoding", "quoted-printable") + + textWriter, err := w.CreatePart(textPart) + if err != nil { + return fmt.Errorf("failed to create text part: %w", err) + } + + qpWriter := quotedprintable.NewWriter(textWriter) + if _, err := qpWriter.Write([]byte(request.PlainText)); err != nil { + qpWriter.Close() + return fmt.Errorf("failed to write text content: %w", err) + } + if err := qpWriter.Close(); err != nil { + return fmt.Errorf("failed to close quoted-printable writer: %w", err) + } + return nil + } + // writeAttachmentPart writes a single attachment (inline or not) as a base64 // MIME part into the given writer. writeAttachmentPart := func(w *multipart.Writer, att domain.Attachment, inline bool) error { @@ -1608,12 +1643,17 @@ func (s *SESService) sendRawEmail(ctx context.Context, sesClient domain.SESv2Cli boundary := writer.Boundary() buf.WriteString(fmt.Sprintf("Content-Type: multipart/mixed; boundary=\"%s\"\r\n\r\n", boundary)) - // HTML body — wrapped in multipart/related when inline images are present. - if len(inlineAtts) > 0 { + // writeHTMLBody writes the HTML body into the given writer — wrapped in + // multipart/related when inline images are present, direct text/html otherwise. + writeHTMLBody := func(w *multipart.Writer) error { + if len(inlineAtts) == 0 { + return writeHTMLPart(w) + } + relatedBoundary := multipart.NewWriter(&bytes.Buffer{}).Boundary() relatedHeader := textproto.MIMEHeader{} relatedHeader.Set("Content-Type", fmt.Sprintf("multipart/related; type=\"text/html\"; boundary=\"%s\"", relatedBoundary)) - relatedPart, err := writer.CreatePart(relatedHeader) + relatedPart, err := w.CreatePart(relatedHeader) if err != nil { return fmt.Errorf("failed to create related part: %w", err) } @@ -1629,11 +1669,35 @@ func (s *SESService) sendRawEmail(ctx context.Context, sesClient domain.SESv2Cli return fmt.Errorf("inline attachment %d: %w", i, err) } } - if err := related.Close(); err != nil { - return fmt.Errorf("failed to close related writer: %w", err) + return related.Close() + } + + // Body — wrapped in multipart/alternative alongside the plain-text part when one + // is set, so the boundary structure matches the no-attachments SendEmail path and + // clients that render only one alternative fall back to plain text, not nothing. + if request.PlainText != "" { + altBoundary := multipart.NewWriter(&bytes.Buffer{}).Boundary() + altHeader := textproto.MIMEHeader{} + altHeader.Set("Content-Type", fmt.Sprintf("multipart/alternative; boundary=\"%s\"", altBoundary)) + altPart, err := writer.CreatePart(altHeader) + if err != nil { + return fmt.Errorf("failed to create alternative part: %w", err) + } + alternative := multipart.NewWriter(altPart) + if err := alternative.SetBoundary(altBoundary); err != nil { + return fmt.Errorf("failed to set alternative boundary: %w", err) + } + if err := writeTextPart(alternative); err != nil { + return err + } + if err := writeHTMLBody(alternative); err != nil { + return err + } + if err := alternative.Close(); err != nil { + return fmt.Errorf("failed to close alternative writer: %w", err) } } else { - if err := writeHTMLPart(writer); err != nil { + if err := writeHTMLBody(writer); err != nil { return err } } diff --git a/internal/service/ses_service_test.go b/internal/service/ses_service_test.go index a9f452ea..d182dcc3 100644 --- a/internal/service/ses_service_test.go +++ b/internal/service/ses_service_test.go @@ -12,13 +12,13 @@ import ( "github.com/Notifuse/notifuse/internal/domain" "github.com/Notifuse/notifuse/internal/domain/mocks" pkgmocks "github.com/Notifuse/notifuse/pkg/mocks" + "github.com/aws/aws-sdk-go-v2/service/sesv2" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ses" - "github.com/aws/aws-sdk-go-v2/service/sesv2" - "github.com/aws/smithy-go" "github.com/aws/aws-sdk-go/service/sns" + "github.com/aws/smithy-go" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1636,6 +1636,94 @@ func TestSendEmail_Success(t *testing.T) { assert.NoError(t, err) } +// TestSendEmail_WithPlainTextAlternative verifies the structured (no-attachment) SES path +// populates Simple.Body.Text alongside Html when a plain-text alternative is set. +func TestSendEmail_WithPlainTextAlternative(t *testing.T) { + service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) + + provider := &domain.EmailProvider{ + SES: &domain.AmazonSESSettings{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + Region: "us-east-1", + }, + } + + mockSESClient.EXPECT(). + ListConfigurationSetsWithContext(gomock.Any(), gomock.Any()). + Return(&ses.ListConfigurationSetsOutput{ConfigurationSets: []*ses.ConfigurationSet{}}, nil) + + mockSESv2.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, input *sesv2.SendEmailInput, _ ...func(*sesv2.Options)) (*sesv2.SendEmailOutput, error) { + require.NotNil(t, input.Content.Simple) + require.NotNil(t, input.Content.Simple.Body.Html) + assert.Equal(t, "Test Content", *input.Content.Simple.Body.Html.Data) + require.NotNil(t, input.Content.Simple.Body.Text) + assert.Equal(t, "Test Content", *input.Content.Simple.Body.Text.Data) + return &sesv2.SendEmailOutput{}, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "test-workspace", + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: "from@example.com", + FromName: "Test Sender", + To: "to@example.com", + Subject: "Test Subject", + Content: "Test Content", + PlainText: "Test Content", + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) +} + +// TestSendEmail_WithoutPlainTextLeavesBodyTextNil verifies the structured SES path leaves +// Simple.Body.Text nil (not an empty string, which SES rejects) when no plain-text alternative +// was provided. +func TestSendEmail_WithoutPlainTextLeavesBodyTextNil(t *testing.T) { + service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) + + provider := &domain.EmailProvider{ + SES: &domain.AmazonSESSettings{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + Region: "us-east-1", + }, + } + + mockSESClient.EXPECT(). + ListConfigurationSetsWithContext(gomock.Any(), gomock.Any()). + Return(&ses.ListConfigurationSetsOutput{ConfigurationSets: []*ses.ConfigurationSet{}}, nil) + + mockSESv2.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, input *sesv2.SendEmailInput, _ ...func(*sesv2.Options)) (*sesv2.SendEmailOutput, error) { + assert.Nil(t, input.Content.Simple.Body.Text) + return &sesv2.SendEmailOutput{}, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "test-workspace", + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: "from@example.com", + FromName: "Test Sender", + To: "to@example.com", + Subject: "Test Subject", + Content: "Test Content", + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) +} + // TestSendEmail_CapturesMessageID verifies SES surfaces the API-returned MessageId via // request.CapturedMessageID, so the worker can store the recipient-visible RFC Message-ID // for stop-on-reply matching (SES overwrites any Message-ID we set). @@ -2412,6 +2500,66 @@ func TestSendEmail_WithInlineAttachment(t *testing.T) { assert.NoError(t, err) } +// TestSendEmail_WithInlineAttachmentAndPlainText verifies the multipart/related subtree +// (HTML + inline image) nests correctly inside multipart/alternative when a plain-text +// alternative is also set — the trickiest of the three nesting levels the raw-MIME path builds. +func TestSendEmail_WithInlineAttachmentAndPlainText(t *testing.T) { + service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) + + provider := &domain.EmailProvider{ + SES: &domain.AmazonSESSettings{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + Region: "us-east-1", + }, + } + + attachments := []domain.Attachment{ + { + Filename: "logo.png", + Content: "iVBORw0KGgo=", + ContentType: "image/png", + Disposition: "inline", + }, + } + + mockSESClient.EXPECT(). + ListConfigurationSetsWithContext(gomock.Any(), gomock.Any()). + Return(&ses.ListConfigurationSetsOutput{}, nil) + + mockSESv2.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, input *sesv2.SendEmailInput, _ ...func(*sesv2.Options)) (*sesv2.SendEmailOutput, error) { + require.NotNil(t, input.Content.Raw) + rawData := string(input.Content.Raw.Data) + + assert.Contains(t, rawData, "Content-Type: multipart/alternative") + assert.Contains(t, rawData, "Content-Type: multipart/related") + assert.Contains(t, rawData, "Content-Type: text/plain; charset=UTF-8") + assert.Contains(t, rawData, "See the logo below") + assert.Contains(t, rawData, "Content-Id: ") + + return &sesv2.SendEmailOutput{}, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace", + IntegrationID: "test-integration-id", + MessageID: "message", + FromAddress: "from@example.com", + FromName: "From", + To: "to@example.com", + Subject: "Subject", + Content: "", + PlainText: "See the logo below", + Provider: provider, + EmailOptions: domain.EmailOptions{Attachments: attachments}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) +} + // Test SendEmail - inline attachment with an explicit content_id must be wrapped // in a multipart/related subtree and carry the caller-provided Content-ID. func TestSendEmail_InlineAttachment_MultipartRelatedAndContentID(t *testing.T) { @@ -3011,6 +3159,123 @@ func TestSendEmail_VerifyMIMEStructureWithAttachments(t *testing.T) { assert.NoError(t, err) } +// TestSendEmail_RawEmail_WithPlainTextAlternative verifies the raw-MIME path (forced here by +// an attachment) wraps the plain-text and HTML bodies in a nested multipart/alternative part, +// itself nested inside the top-level multipart/mixed alongside the attachment. +func TestSendEmail_RawEmail_WithPlainTextAlternative(t *testing.T) { + service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) + + provider := &domain.EmailProvider{ + SES: &domain.AmazonSESSettings{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + Region: "us-east-1", + }, + } + + attachments := []domain.Attachment{ + { + Filename: "test.txt", + Content: "SGVsbG8gV29ybGQ=", // "Hello World" + ContentType: "text/plain", + Disposition: "attachment", + }, + } + + mockSESClient.EXPECT(). + ListConfigurationSetsWithContext(gomock.Any(), gomock.Any()). + Return(&ses.ListConfigurationSetsOutput{}, nil) + + mockSESv2.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, input *sesv2.SendEmailInput, _ ...func(*sesv2.Options)) (*sesv2.SendEmailOutput, error) { + require.NotNil(t, input.Content.Raw) + rawData := string(input.Content.Raw.Data) + + assert.Contains(t, rawData, "Content-Type: multipart/mixed") + assert.Contains(t, rawData, "Content-Type: multipart/alternative") + assert.Contains(t, rawData, "Content-Type: text/plain; charset=UTF-8") + assert.Contains(t, rawData, "Content-Type: text/html; charset=UTF-8") + assert.Contains(t, rawData, "Plain text body") + assert.Contains(t, rawData, "Test") + assert.Contains(t, rawData, "Content-Disposition: attachment; filename=\"test.txt\"") + + return &sesv2.SendEmailOutput{}, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace", + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: "from@example.com", + FromName: "From", + To: "to@example.com", + Subject: "Test Subject", + Content: "Test", + PlainText: "Plain text body", + Provider: provider, + EmailOptions: domain.EmailOptions{Attachments: attachments}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) +} + +// TestSendEmail_RawEmail_WithoutPlainTextSkipsAlternative verifies the raw-MIME path keeps its +// original (non-nested) structure when no plain-text alternative is set. +func TestSendEmail_RawEmail_WithoutPlainTextSkipsAlternative(t *testing.T) { + service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) + + provider := &domain.EmailProvider{ + SES: &domain.AmazonSESSettings{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + Region: "us-east-1", + }, + } + + attachments := []domain.Attachment{ + { + Filename: "test.txt", + Content: "SGVsbG8gV29ybGQ=", + ContentType: "text/plain", + Disposition: "attachment", + }, + } + + mockSESClient.EXPECT(). + ListConfigurationSetsWithContext(gomock.Any(), gomock.Any()). + Return(&ses.ListConfigurationSetsOutput{}, nil) + + mockSESv2.EXPECT(). + SendEmail(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, input *sesv2.SendEmailInput, _ ...func(*sesv2.Options)) (*sesv2.SendEmailOutput, error) { + require.NotNil(t, input.Content.Raw) + rawData := string(input.Content.Raw.Data) + + assert.NotContains(t, rawData, "multipart/alternative") + assert.Contains(t, rawData, "Content-Type: text/html; charset=UTF-8") + + return &sesv2.SendEmailOutput{}, nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace", + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: "from@example.com", + FromName: "From", + To: "to@example.com", + Subject: "Test Subject", + Content: "Test", + Provider: provider, + EmailOptions: domain.EmailOptions{Attachments: attachments}, + } + err := service.SendEmail(context.Background(), request) + + assert.NoError(t, err) +} + // Test SendEmail - with List-Unsubscribe headers (RFC-8058) func TestSendEmail_WithListUnsubscribeHeaders(t *testing.T) { service, mockSESClient, _, _, _, mockSESv2 := createMockSESServiceWithV2(t) diff --git a/internal/service/smtp_service.go b/internal/service/smtp_service.go index 1fad2b8f..fb2a7f60 100644 --- a/internal/service/smtp_service.go +++ b/internal/service/smtp_service.go @@ -620,7 +620,14 @@ func (s *SMTPService) SendEmail(ctx context.Context, request domain.SendEmailPro } msg.Subject(request.Subject) - msg.SetBodyString(mail.TypeTextHTML, request.Content) + if request.PlainText != "" { + // Ordered least-to-most-preferred per RFC 2046 §5.1.4: text/plain first, + // text/html as the alternative, so clients that render only one part pick HTML. + msg.SetBodyString(mail.TypeTextPlain, request.PlainText) + msg.AddAlternativeString(mail.TypeTextHTML, request.Content) + } else { + msg.SetBodyString(mail.TypeTextHTML, request.Content) + } // Add attachments if specified for i, att := range request.EmailOptions.Attachments { diff --git a/internal/service/smtp_service_test.go b/internal/service/smtp_service_test.go index 4cc5c7cf..af0df930 100644 --- a/internal/service/smtp_service_test.go +++ b/internal/service/smtp_service_test.go @@ -730,6 +730,88 @@ func TestSMTPService_SendEmail_Integration(t *testing.T) { assert.NotContains(t, mailFromCmd, "SMTPUTF8") } +func TestSMTPService_SendEmail_WithPlainTextAlternative(t *testing.T) { + server := newMockSMTPServer(t, true) + defer server.Close() + + log := &noopLogger{} + service := NewSMTPService(log) + + provider := &domain.EmailProvider{ + Kind: domain.EmailProviderKindSMTP, + SMTP: &domain.SMTPSettings{ + Host: "127.0.0.1", + Port: server.Port(), + }, + } + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace-123", + IntegrationID: "integration-123", + MessageID: "message-123", + FromAddress: "sender@example.com", + FromName: "Test Sender", + To: "recipient@example.com", + Subject: "Test Subject", + Content: "

Hello

This is a test.

", + PlainText: "Hello\n\nThis is a test.", + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + + err := service.SendEmail(context.Background(), request) + require.NoError(t, err) + + messages := server.GetMessages() + require.Len(t, messages, 1) + + body := string(messages[0].data) + assert.Contains(t, body, "multipart/alternative") + assert.Contains(t, body, "text/plain") + assert.Contains(t, body, "text/html") + assert.Contains(t, body, "This is a test.") + assert.Contains(t, body, "Hello") +} + +func TestSMTPService_SendEmail_WithoutPlainTextIsHTMLOnly(t *testing.T) { + server := newMockSMTPServer(t, true) + defer server.Close() + + log := &noopLogger{} + service := NewSMTPService(log) + + provider := &domain.EmailProvider{ + Kind: domain.EmailProviderKindSMTP, + SMTP: &domain.SMTPSettings{ + Host: "127.0.0.1", + Port: server.Port(), + }, + } + + request := domain.SendEmailProviderRequest{ + WorkspaceID: "workspace-123", + IntegrationID: "integration-123", + MessageID: "message-123", + FromAddress: "sender@example.com", + FromName: "Test Sender", + To: "recipient@example.com", + Subject: "Test Subject", + Content: "

Hello

This is a test.

", + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + + err := service.SendEmail(context.Background(), request) + require.NoError(t, err) + + messages := server.GetMessages() + require.Len(t, messages, 1) + + body := string(messages[0].data) + assert.NotContains(t, body, "multipart/alternative") + assert.Contains(t, body, "text/html") +} + func TestSMTPService_SendEmail_DefaultEhloUsesFromDomain(t *testing.T) { server := newMockSMTPServer(t, true) defer server.Close() diff --git a/internal/service/sparkpost_service.go b/internal/service/sparkpost_service.go index 884e1634..6260fca5 100644 --- a/internal/service/sparkpost_service.go +++ b/internal/service/sparkpost_service.go @@ -810,6 +810,7 @@ func (s *SparkPostService) SendEmail(ctx context.Context, request domain.SendEma Subject string `json:"subject"` ReplyTo string `json:"reply_to,omitempty"` HTML string `json:"html"` + Text string `json:"text,omitempty"` Headers map[string]string `json:"headers,omitempty"` Attachments []Attachment `json:"attachments,omitempty"` InlineImages []InlineImage `json:"inline_images,omitempty"` @@ -842,6 +843,7 @@ func (s *SparkPostService) SendEmail(ctx context.Context, request domain.SendEma }, Subject: request.Subject, HTML: request.Content, + Text: request.PlainText, }, Metadata: map[string]interface{}{ "notifuse_message_id": request.MessageID, diff --git a/internal/service/sparkpost_service_test.go b/internal/service/sparkpost_service_test.go index e0b257ba..116289da 100644 --- a/internal/service/sparkpost_service_test.go +++ b/internal/service/sparkpost_service_test.go @@ -12,6 +12,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/Notifuse/notifuse/internal/domain" "github.com/Notifuse/notifuse/internal/domain/mocks" @@ -1072,6 +1073,50 @@ func TestSparkPostService_SendEmail(t *testing.T) { assert.NoError(t, err) }) + t.Run("Success with plain text alternative", func(t *testing.T) { + ctx := context.Background() + plainText := "Test Email Content" + + provider := &domain.EmailProvider{ + SparkPost: &domain.SparkPostSettings{ + Endpoint: "https://api.sparkpost.test", + APIKey: "test-api-key", + }, + } + + mockHTTPClient.EXPECT(). + Do(gomock.Any()). + DoAndReturn(func(req *http.Request) (*http.Response, error) { + body, _ := io.ReadAll(req.Body) + var emailReq map[string]interface{} + require.NoError(t, json.Unmarshal(body, &emailReq)) + + contentMap, ok := emailReq["content"].(map[string]interface{}) + assert.True(t, ok) + assert.Equal(t, content, contentMap["html"]) + assert.Equal(t, plainText, contentMap["text"]) + + return mockHTTPResponse(http.StatusOK, `{"results":{"id":"test-transmission-id"}}`), nil + }) + + request := domain.SendEmailProviderRequest{ + WorkspaceID: workspaceID, + IntegrationID: "test-integration-id", + MessageID: "test-message-id", + FromAddress: fromAddress, + FromName: fromName, + To: to, + Subject: subject, + Content: content, + PlainText: plainText, + Provider: provider, + EmailOptions: domain.EmailOptions{}, + } + err := sparkPostService.SendEmail(ctx, request) + + assert.NoError(t, err) + }) + t.Run("Missing SparkPost configuration", func(t *testing.T) { ctx := context.Background()