fixed the notification service error when the user's display name con…

changelog/unreleased/fix-email-notification.md

@@ -0,0 +1,7 @@
+Bugfix: Fix the email notification service
+
+We fixed an error in the notification service that caused the email notification to fail when the user's display name contained special characters.
+
+
+https://github.com/owncloud/ocis/pull/9514
+https://github.com/owncloud/ocis/issues/9402

services/notifications/pkg/channels/channels.go

@@ -4,7 +4,7 @@ package channels
 import (
 	"context"
 	"crypto/tls"
-	"fmt"
+	stdmail "net/mail"
 	"strings"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
@@ -31,16 +31,24 @@ type Message struct {
 
 // NewMailChannel instantiates a new mail communication channel.
 func NewMailChannel(cfg config.Config, logger log.Logger) (Channel, error) {
+	var err error
+	var a *stdmail.Address
+	a, err = stdmail.ParseAddress(cfg.Notifications.SMTP.Sender)
+	if err != nil {
+		logger.Err(err).Msg("parsing error, the 'smtp_sender' must be a valid single RFC 5322 address.")
+	}
 	return Mail{
-		conf:   cfg,
-		logger: logger,
+		conf:        cfg,
+		smtpAddress: a,
+		logger:      logger,
 	}, nil
 }
 
 // Mail is the communication channel for email.
 type Mail struct {
-	conf   config.Config
-	logger log.Logger
+	conf        config.Config
+	smtpAddress *stdmail.Address
+	logger      log.Logger
 }
 
 func (m Mail) getMailClient() (*mail.SMTPClient, error) {
@@ -102,8 +110,12 @@ func (m Mail) getMailClient() (*mail.SMTPClient, error) {
 
 // SendMessage sends a message to all given users.
 func (m Mail) SendMessage(ctx context.Context, message *Message) error {
+	if m.smtpAddress == nil {
+		m.logger.Error().Str("mail", "SendMessage").Msg("failed to send a message. SMTP address is not set")
+		return nil
+	}
 	if m.conf.Notifications.SMTP.Host == "" {
-		m.logger.Info().Str("mail", "SendMessage").Msg("failed to send a message. SMTP host is  not set")
+		m.logger.Info().Str("mail", "SendMessage").Msg("failed to send a message. SMTP host is not set")
 		return nil
 	}
 
@@ -113,11 +125,7 @@ func (m Mail) SendMessage(ctx context.Context, message *Message) error {
 	}
 
 	email := mail.NewMSG()
-	if message.Sender != "" {
-		email.SetFrom(fmt.Sprintf("%s via %s", message.Sender, m.conf.Notifications.SMTP.Sender)).AddTo(message.Recipient...)
-	} else {
-		email.SetFrom(m.conf.Notifications.SMTP.Sender).AddTo(message.Recipient...)
-	}
+	email.SetFrom(appendSender(message.Sender, *m.smtpAddress)).AddTo(message.Recipient...)
 	email.SetSubject(message.Subject)
 	email.SetBody(mail.TextPlain, message.TextBody)
 	if message.HTMLBody != "" {
@@ -129,3 +137,10 @@ func (m Mail) SendMessage(ctx context.Context, message *Message) error {
 
 	return email.Send(smtpClient)
 }
+
+func appendSender(sender string, a stdmail.Address) string {
+	if strings.TrimSpace(sender) != "" {
+		a.Name = strings.TrimSpace(sender + " via " + a.Name)
+	}
+	return a.String()
+}

services/notifications/pkg/channels/channels_test.go

@@ -0,0 +1,64 @@
+package channels
+
+import (
+	"net/mail"
+	"testing"
+)
+
+func Test_appendSender(t *testing.T) {
+	type args struct {
+		sender string
+		a      mail.Address
+	}
+
+	a1, err := mail.ParseAddress("ownCloud <[email protected]>")
+	if err != nil {
+		t.Error(err)
+	}
+	a2, err := mail.ParseAddress("[email protected]")
+	if err != nil {
+		t.Error(err)
+	}
+
+	tests := []struct {
+		name   string
+		sender string
+		want1  string
+		want2  string
+	}{
+		{
+			name:   "empty sender",
+			sender: "",
+			want1:  `"ownCloud" <[email protected]>`,
+			want2:  `<[email protected]>`,
+		},
+		{
+			name:   "not empty sender",
+			sender: `Joe Q. Public`,
+			want1:  `"Joe Q. Public via ownCloud" <[email protected]>`,
+			want2:  `"Joe Q. Public via" <[email protected]>`,
+		},
+		{
+			name:   "sender whit comma and semicolon",
+			sender: `Joe, Q; Public:`,
+			want1:  `"Joe, Q; Public: via ownCloud" <[email protected]>`,
+			want2:  `"Joe, Q; Public: via" <[email protected]>`,
+		},
+		{
+			name:   "sender with quotes",
+			sender: `Joe Q. "Public"`,
+			want1:  `"Joe Q. \"Public\" via ownCloud" <[email protected]>`,
+			want2:  `"Joe Q. \"Public\" via" <[email protected]>`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := appendSender(tt.sender, *a1); got != tt.want1 {
+				t.Errorf("appendSender() = %v, want %v", got, tt.want1)
+			}
+			if got := appendSender(tt.sender, *a2); got != tt.want2 {
+				t.Errorf("appendSender() = %v, want %v", got, tt.want2)
+			}
+		})
+	}
+}

services/notifications/pkg/config/parser/parse.go

@@ -3,6 +3,8 @@ package parser
 import (
 	"errors"
 	"fmt"
+	"net/mail"
+	"strings"
 
 	ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config"
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
@@ -54,6 +56,14 @@ func Validate(cfg *config.Config) error {
 		}
 	}
 
+	if strings.TrimSpace(cfg.Notifications.SMTP.Sender) != "" {
+		if s, err := mail.ParseAddress(cfg.Notifications.SMTP.Sender); err == nil {
+			cfg.Notifications.SMTP.Sender = s.String()
+		} else {
+			return fmt.Errorf("the 'smtp_sender' must be a valid single RFC 5322 address. parsing error %w", err)
+		}
+	}
+
 	if cfg.ServiceAccount.ServiceAccountID == "" {
 		return shared.MissingServiceAccountID(cfg.Service.Name)
 	}