Skip to content

Conversation

@bmonkman
Copy link
Contributor

A comma separated list of domains can now be specified as an env var, and email sending (including to, cc, bcc) will be restricted to only addresses in those domains.


@bmonkman bmonkman requested a review from a team as a code owner March 12, 2021 00:44
@bmonkman bmonkman requested a review from davidcheung March 12, 2021 00:44
@bmonkman bmonkman requested a review from timothy-wan March 12, 2021 00:53
func RemoveInvalidRecipients(recipients []server.EmailRecipient, allowedDomains []string) []server.EmailRecipient {
valid := []server.EmailRecipient{}
for _, recipient := range recipients {

Copy link
Contributor

@pthieu pthieu Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary newline

Copy link
Contributor

@davidcheung davidcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think RESTRICT_EMAIL_TO_DOMAINS might be a little more clear, but with the description i think is good enough

@bmonkman
Copy link
Contributor Author

That's what I used originally, but I changed it because I found "Restrict" to be a little more ambiguous. Does it mean emails are restricted from going to those domains or restricted to going to anything that's not those domains?

@pthieu
Copy link
Contributor

pthieu commented Mar 12, 2021

How about ALLOWED_DOMAINS?

@bmonkman
Copy link
Contributor Author

Right now it's ALLOW_EMAIL_TO_DOMAINS, and it needs to be specific to email, as the service handles things other than email.

1 similar comment
@bmonkman
Copy link
Contributor Author

Right now it's ALLOW_EMAIL_TO_DOMAINS, and it needs to be specific to email, as the service handles things other than email.

@davidcheung
Copy link
Contributor

👍 Yeah i think either way once the user reads the description the behavior will be apparent,
and if they dont then 🤷

@bmonkman bmonkman merged commit f902186 into main Mar 12, 2021
@bmonkman bmonkman deleted the configurable-email-domain-restrictions branch March 12, 2021 20:44
dtoki pushed a commit to dtoki/zero-notification-service that referenced this pull request Apr 9, 2021
…y send mail to addresses in those domains (commitdev#21)

* enhancement: Added the ability to configure a list of domains and only send mail to addresses in those domains

* Added env var docs

* Updated helm chart
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants