From f29b2543333d8ba56e29474205a22de6527ae379 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 25 Jun 2024 17:51:51 +0200 Subject: [PATCH] fixed the notification service error when the user's display name contained special characters --- .drone.star | 1 + .../unreleased/fix-email-notification.md | 6 ++ deployments/examples/ocis_full/inbucket.yml | 2 + docs/ocis/development/testing.md | 1 + ocis/pkg/runtime/service/service.go | 12 ++-- .../notifications/pkg/channels/channels.go | 30 ++++++--- .../pkg/channels/channels_test.go | 64 +++++++++++++++++++ .../notifications/pkg/config/parser/parse.go | 13 ++++ 8 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 changelog/unreleased/fix-email-notification.md create mode 100644 services/notifications/pkg/channels/channels_test.go diff --git a/.drone.star b/.drone.star index c8116afd1..6560d132c 100644 --- a/.drone.star +++ b/.drone.star @@ -108,6 +108,7 @@ config = { "EMAIL_PORT": "9000", }, "extraServerEnvironment": { + "OCIS_ADD_RUN_SERVICES": "notifications", "NOTIFICATIONS_SMTP_HOST": "email", "NOTIFICATIONS_SMTP_PORT": "2500", "NOTIFICATIONS_SMTP_INSECURE": "true", diff --git a/changelog/unreleased/fix-email-notification.md b/changelog/unreleased/fix-email-notification.md new file mode 100644 index 000000000..0edc0a592 --- /dev/null +++ b/changelog/unreleased/fix-email-notification.md @@ -0,0 +1,6 @@ +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/9467 +https://github.com/owncloud/ocis/issues/9402 diff --git a/deployments/examples/ocis_full/inbucket.yml b/deployments/examples/ocis_full/inbucket.yml index 47e1d2889..495006edf 100644 --- a/deployments/examples/ocis_full/inbucket.yml +++ b/deployments/examples/ocis_full/inbucket.yml @@ -2,6 +2,8 @@ services: ocis: environment: + # enable the notifications service + OCIS_ADD_RUN_SERVICES": "notifications" NOTIFICATIONS_SMTP_HOST: inbucket NOTIFICATIONS_SMTP_PORT: 2500 NOTIFICATIONS_SMTP_SENDER: oCIS notifications diff --git a/docs/ocis/development/testing.md b/docs/ocis/development/testing.md index 3c10257a4..b252bba26 100644 --- a/docs/ocis/development/testing.md +++ b/docs/ocis/development/testing.md @@ -359,6 +359,7 @@ ocis/bin/ocis init --insecure true # run oCIS PROXY_ENABLE_BASIC_AUTH=true \ +OCIS_ADD_RUN_SERVICES=notifications \ NOTIFICATIONS_SMTP_HOST=localhost \ NOTIFICATIONS_SMTP_PORT=2500 \ NOTIFICATIONS_SMTP_INSECURE=true \ diff --git a/ocis/pkg/runtime/service/service.go b/ocis/pkg/runtime/service/service.go index a4f3faa2f..7c8ec280f 100644 --- a/ocis/pkg/runtime/service/service.go +++ b/ocis/pkg/runtime/service/service.go @@ -19,6 +19,7 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/mohae/deepcopy" "github.com/olekukonko/tablewriter" + notifications "github.com/owncloud/ocis/v2/services/notifications/pkg/command" "github.com/thejerf/suture/v4" ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config" @@ -43,7 +44,6 @@ import ( idp "github.com/owncloud/ocis/v2/services/idp/pkg/command" invitations "github.com/owncloud/ocis/v2/services/invitations/pkg/command" nats "github.com/owncloud/ocis/v2/services/nats/pkg/command" - notifications "github.com/owncloud/ocis/v2/services/notifications/pkg/command" ocdav "github.com/owncloud/ocis/v2/services/ocdav/pkg/command" ocm "github.com/owncloud/ocis/v2/services/ocm/pkg/command" ocs "github.com/owncloud/ocis/v2/services/ocs/pkg/command" @@ -200,11 +200,6 @@ func NewService(options ...Option) (*Service, error) { cfg.IDM.Commons = cfg.Commons return idm.Execute(cfg.IDM) }) - reg(3, opts.Config.Notifications.Service.Name, func(ctx context.Context, cfg *ociscfg.Config) error { - cfg.Notifications.Context = ctx - cfg.Notifications.Commons = cfg.Commons - return notifications.Execute(cfg.Notifications) - }) reg(3, opts.Config.OCDav.Service.Name, func(ctx context.Context, cfg *ociscfg.Config) error { cfg.OCDav.Context = ctx cfg.OCDav.Commons = cfg.Commons @@ -339,6 +334,11 @@ func NewService(options ...Option) (*Service, error) { cfg.Invitations.Commons = cfg.Commons return invitations.Execute(cfg.Invitations) }) + areg(opts.Config.Notifications.Service.Name, func(ctx context.Context, cfg *ociscfg.Config) error { + cfg.Notifications.Context = ctx + cfg.Notifications.Commons = cfg.Commons + return notifications.Execute(cfg.Notifications) + }) return s, nil } diff --git a/services/notifications/pkg/channels/channels.go b/services/notifications/pkg/channels/channels.go index d8d368a63..5ac537e3f 100644 --- a/services/notifications/pkg/channels/channels.go +++ b/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,23 @@ type Message struct { // NewMailChannel instantiates a new mail communication channel. func NewMailChannel(cfg config.Config, logger log.Logger) (Channel, error) { + 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 nil, err + } 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) { @@ -113,11 +120,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 +132,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() +} diff --git a/services/notifications/pkg/channels/channels_test.go b/services/notifications/pkg/channels/channels_test.go new file mode 100644 index 000000000..5140859d9 --- /dev/null +++ b/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 ") + if err != nil { + t.Error(err) + } + a2, err := mail.ParseAddress("noreply@example.com") + if err != nil { + t.Error(err) + } + + tests := []struct { + name string + sender string + want1 string + want2 string + }{ + { + name: "empty sender", + sender: "", + want1: `"ownCloud" `, + want2: ``, + }, + { + name: "not empty sender", + sender: `Joe Q. Public`, + want1: `"Joe Q. Public via ownCloud" `, + want2: `"Joe Q. Public via" `, + }, + { + name: "sender whit comma and semicolon", + sender: `Joe, Q; Public:`, + want1: `"Joe, Q; Public: via ownCloud" `, + want2: `"Joe, Q; Public: via" `, + }, + { + name: "sender with quotes", + sender: `Joe Q. "Public"`, + want1: `"Joe Q. \"Public\" via ownCloud" `, + want2: `"Joe Q. \"Public\" via" `, + }, + } + 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) + } + }) + } +} diff --git a/services/notifications/pkg/config/parser/parse.go b/services/notifications/pkg/config/parser/parse.go index 032d2a6b3..c1beebca0 100644 --- a/services/notifications/pkg/config/parser/parse.go +++ b/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,17 @@ func Validate(cfg *config.Config) error { } } + { + if strings.TrimSpace(cfg.Notifications.SMTP.Sender) == "" { + return fmt.Errorf("the 'smtp_sender' must be a valid single RFC 5322 address. parsing error: the address cannot be empty") + } + 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) }