From 8a7673613d4b3d696cfbe6b58bc11f40a97e59c6 Mon Sep 17 00:00:00 2001 From: Bill Monkman Date: Thu, 11 Mar 2021 16:43:28 -0800 Subject: [PATCH 1/3] enhancement: Added the ability to configure a list of domains and only send mail to addresses in those domains --- internal/config/config.go | 6 ++++ internal/mail/mail.go | 24 +++++++++++++++ internal/mail/mail_test.go | 44 +++++++++++++++++++-------- internal/service/api_email_service.go | 28 +++++++++++++++++ 4 files changed, 90 insertions(+), 12 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index a6cfc7b..5250e5f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -13,6 +13,7 @@ type Config struct { SlackAPIKey string GracefulShutdownTimeout time.Duration StructuredLogging bool + AllowEmailToDomains []string } var config *Config @@ -24,6 +25,7 @@ const ( SlackAPIKey GracefulShutdownTimeout StructuredLogging + AllowEmailToDomains ) // GetConfig returns a pointer to the singleton Config object @@ -50,12 +52,16 @@ func loadConfig() *Config { viper.SetDefault(StructuredLogging, "false") viper.BindEnv(StructuredLogging, "STRUCTURED_LOGGING") + viper.SetDefault(AllowEmailToDomains, []string{}) + viper.BindEnv(AllowEmailToDomains, "ALLOW_EMAIL_TO_DOMAINS") + config := Config{ Port: viper.GetInt(Port), SendgridAPIKey: viper.GetString(SendgridAPIKey), SlackAPIKey: viper.GetString(SlackAPIKey), GracefulShutdownTimeout: viper.GetDuration(GracefulShutdownTimeout), StructuredLogging: viper.GetBool(StructuredLogging), + AllowEmailToDomains: viper.GetStringSlice(AllowEmailToDomains), } return &config diff --git a/internal/mail/mail.go b/internal/mail/mail.go index 50f4d94..503c869 100644 --- a/internal/mail/mail.go +++ b/internal/mail/mail.go @@ -1,6 +1,8 @@ package mail import ( + "fmt" + "strings" "sync" "github.com/commitdev/zero-notification-service/internal/server" @@ -81,3 +83,25 @@ func convertAddresses(addresses []server.EmailRecipient) []*sendgridMail.Email { } return returnAddresses } + +// RemoveInvalidRecipients accepts a list of recipients and removes the ones with domains not in the allowed list +func RemoveInvalidRecipients(recipients []server.EmailRecipient, allowedDomains []string) []server.EmailRecipient { + valid := []server.EmailRecipient{} + for _, recipient := range recipients { + + if addressInAllowedDomain(recipient.Address, allowedDomains) { + valid = append(valid, recipient) + } + } + return valid +} + +// addressInAllowedDomain checks if a single email address is in a list of domains +func addressInAllowedDomain(address string, domains []string) bool { + for _, domain := range domains { + if strings.HasSuffix(address, fmt.Sprintf("@%s", domain)) { + return true + } + } + return false +} diff --git a/internal/mail/mail_test.go b/internal/mail/mail_test.go index 1270537..205cb4f 100644 --- a/internal/mail/mail_test.go +++ b/internal/mail/mail_test.go @@ -25,16 +25,7 @@ func (cl *FakeClient) Send(email *sendgridMail.SGMailV3) (*rest.Response, error) } func TestSendBulkMail(t *testing.T) { - var toList []server.EmailRecipient - // Create a random number of mails - rand.Seed(time.Now().UnixNano()) - sendCount := rand.Intn(5) + 2 - for i := 0; i < sendCount; i++ { - toList = append(toList, server.EmailRecipient{ - Name: fmt.Sprintf("Test Recipient %d", i), - Address: fmt.Sprintf("address%d@domain.com", i), - }) - } + toList := createRandomRecipients(2, 5) cc := make([]server.EmailRecipient, 0) bcc := make([]server.EmailRecipient, 0) from := server.EmailSender{Name: "Test User", Address: "address@domain.com"} @@ -52,8 +43,37 @@ func TestSendBulkMail(t *testing.T) { returnedCount++ } - assert.Equal(t, sendCount, returnedCount, "Response count should match requests sent") + assert.Equal(t, len(toList), returnedCount, "Response count should match requests sent") // Check that the send function was called - client.AssertNumberOfCalls(t, "Send", sendCount) + client.AssertNumberOfCalls(t, "Send", len(toList)) +} + +func TestRemoveInvalidRecipients(t *testing.T) { + toList := createRandomRecipients(2, 5) + + originalSize := len(toList) + + toList[0].Address = "address@commit.dev" + + alteredList := mail.RemoveInvalidRecipients(toList, []string{"commit.dev", "domain.com"}) + assert.Equal(t, len(alteredList), originalSize, "All addresses should remain in the list") + + alteredList = mail.RemoveInvalidRecipients(toList, []string{"commit.dev"}) + assert.Equal(t, len(alteredList), 1, "1 address should remain in the list") +} + +// createRandomRecipients creates a random list of recipients +func createRandomRecipients(min int, randCount int) []server.EmailRecipient { + var toList []server.EmailRecipient + // Create a random number of mails + rand.Seed(time.Now().UnixNano()) + sendCount := rand.Intn(randCount) + min + for i := 0; i < sendCount; i++ { + toList = append(toList, server.EmailRecipient{ + Name: fmt.Sprintf("Test Recipient %d", i), + Address: fmt.Sprintf("address%d@domain.com", i), + }) + } + return toList } diff --git a/internal/service/api_email_service.go b/internal/service/api_email_service.go index c3be60d..b99b07d 100644 --- a/internal/service/api_email_service.go +++ b/internal/service/api_email_service.go @@ -26,6 +26,20 @@ func NewEmailApiService(c *config.Config) server.EmailApiServicer { // SendEmail - Send an email func (s *EmailApiService) SendEmail(ctx context.Context, sendMailRequest server.SendMailRequest) (server.ImplResponse, error) { + + // Check if there are valid recipients who are not in the restriction list + if len(s.config.AllowEmailToDomains) > 0 { + originalAddresses := sendMailRequest.ToAddresses + sendMailRequest.ToAddresses = mail.RemoveInvalidRecipients(sendMailRequest.ToAddresses, s.config.AllowEmailToDomains) + // If there are none, return with a 200 but don't send anything + if len(sendMailRequest.ToAddresses) == 0 { + zap.S().Infow("No valid Recipients for send", zap.Any("original_addresses", originalAddresses)) + return server.Response(http.StatusOK, server.SendMailResponse{TrackingId: "No valid recipients"}), nil + } + } + sendMailRequest.CcAddresses = mail.RemoveInvalidRecipients(sendMailRequest.CcAddresses, s.config.AllowEmailToDomains) + sendMailRequest.BccAddresses = mail.RemoveInvalidRecipients(sendMailRequest.BccAddresses, s.config.AllowEmailToDomains) + client := sendgrid.NewSendClient(s.config.SendgridAPIKey) response, err := mail.SendIndividualMail(sendMailRequest.ToAddresses, sendMailRequest.FromAddress, sendMailRequest.CcAddresses, sendMailRequest.BccAddresses, sendMailRequest.Message, client) @@ -45,6 +59,20 @@ func (s *EmailApiService) SendEmail(ctx context.Context, sendMailRequest server. // SendBulk - Send a batch of emails to many users with the same content. Note that it is possible for only a subset of these to fail. func (s *EmailApiService) SendBulk(ctx context.Context, sendBulkMailRequest server.SendBulkMailRequest) (server.ImplResponse, error) { + // Check if there are valid recipients who are not in the restriction list + if len(s.config.AllowEmailToDomains) > 0 { + originalAddresses := sendBulkMailRequest.ToAddresses + sendBulkMailRequest.ToAddresses = mail.RemoveInvalidRecipients(sendBulkMailRequest.ToAddresses, s.config.AllowEmailToDomains) + + // If there are none, return with a 200 but don't send anything + if len(sendBulkMailRequest.ToAddresses) == 0 { + zap.S().Infow("No valid Recipients for bulk send", zap.Any("original_addresses", originalAddresses)) + return server.Response(http.StatusOK, server.SendMailResponse{TrackingId: "No valid recipients"}), nil + } + } + sendBulkMailRequest.CcAddresses = mail.RemoveInvalidRecipients(sendBulkMailRequest.CcAddresses, s.config.AllowEmailToDomains) + sendBulkMailRequest.BccAddresses = mail.RemoveInvalidRecipients(sendBulkMailRequest.BccAddresses, s.config.AllowEmailToDomains) + client := sendgrid.NewSendClient(s.config.SendgridAPIKey) responseChannel := make(chan mail.BulkSendAttempt) From 2a800575b8dde6a9146e5de6da80096218709c2d Mon Sep 17 00:00:00 2001 From: Bill Monkman Date: Thu, 11 Mar 2021 16:53:02 -0800 Subject: [PATCH 2/3] Added env var docs --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index 58595b8..bdda4fb 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,18 @@ make docker make docker-run ``` +### Configuration +A number of environment variables are available to configure the service at runtime: +| Env var name | Functionality | Default | +|--------------|---------------|---------| +| SERVICE_PORT | The local port the application will bind to | 80 | +| SENDGRID_API_KEY | The API Key for sendgrid | | +| SLACK_API_KEY | The API Token for Slack | | +| GRACEFUL_SHUTDOWN_TIMEOUT_SECONDS | The number of seconds the application will continue servicing in-flight requests before the application stops after it receives an interrupt signal | 10 | +| STRUCTURED_LOGGING | If enabled, logs will be in JSON format, and only above INFO level | false | +| ALLOW_EMAIL_TO_DOMAINS | A comma separated list of domains. Only addresses in this list can have email sent to them. If empty, disable this "sandboxing" functionality. | | + + ### Releasing a new version on GitHub and Brew We are using a tool called `goreleaser` which you can get from brew if you're on MacOS: From 7b8676153177f852895e2bac74c14211f05f85f3 Mon Sep 17 00:00:00 2001 From: Bill Monkman Date: Thu, 11 Mar 2021 16:57:38 -0800 Subject: [PATCH 3/3] Updated helm chart --- charts/zero-notifcation-service/Chart.yaml | 4 ++-- charts/zero-notifcation-service/templates/configmap.yaml | 1 + charts/zero-notifcation-service/values.yaml | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/charts/zero-notifcation-service/Chart.yaml b/charts/zero-notifcation-service/Chart.yaml index ad49e4c..d063a19 100644 --- a/charts/zero-notifcation-service/Chart.yaml +++ b/charts/zero-notifcation-service/Chart.yaml @@ -15,9 +15,9 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.0.5 +version: 0.0.6 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. -appVersion: 0.0.5 +appVersion: 0.0.9 diff --git a/charts/zero-notifcation-service/templates/configmap.yaml b/charts/zero-notifcation-service/templates/configmap.yaml index 6402976..75ee532 100644 --- a/charts/zero-notifcation-service/templates/configmap.yaml +++ b/charts/zero-notifcation-service/templates/configmap.yaml @@ -6,3 +6,4 @@ data: SERVICE_PORT: "{{ .Values.service.port }}" GRACEFUL_SHUTDOWN_TIMEOUT_SECONDS: "{{ .Values.application.gracefulShutdownTimeout }}" STRUCTURED_LOGGING: "{{ .Values.application.structuredLogging }}" + ALLOW_EMAIL_TO_DOMAINS: "{{ .Values.application.allowEmailToDomains }}" diff --git a/charts/zero-notifcation-service/values.yaml b/charts/zero-notifcation-service/values.yaml index 30c87ed..beb7f2c 100644 --- a/charts/zero-notifcation-service/values.yaml +++ b/charts/zero-notifcation-service/values.yaml @@ -76,9 +76,11 @@ affinity: {} # Application Config +# See project readme for more information about config options application: sendgridApiKey: slackApiKey: gracefulShutdownTimeout: 10 structuredLogging: true + allowEmailToDomains: