Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Recipient allowlist for SMTP relay configuration #109

Merged

Conversation

gliwka
Copy link
Contributor

@gliwka gliwka commented May 4, 2023

Implements the recipient allowlist discussed in #108

I've kept the UI part simple for now by just displaying the configured allowlist whenever it's active and denying the release on the backend. I could not reuse the regex on the frontend since golang and javascript have a different regex syntax:

Screenshot 2023-05-04 at 23 50 15

Screenshot 2023-05-04 at 23 51 00

@axllent axllent changed the base branch from develop to feature/recipient-allowlist May 5, 2023 03:24
@axllent axllent changed the title Recipient allowlist Feature: Recipient allowlist for SMTP relay configuration May 5, 2023
@axllent axllent merged commit fdc1b05 into axllent:feature/recipient-allowlist May 5, 2023
@axllent
Copy link
Owner

axllent commented May 5, 2023

This is a really great effort, so thank you so much, I love it! I think the way you've handled the UI error is about as good as it is going to get - we need API errors anyway, and I can't see how to integrate this into JS for the same reason you pointed out (different syntax).

There was a minor debug issue whereby a "rejected" email still appears to have been sent simply because Send() did not return an error when there were no valid recipients (despite nothing being sent), eg:

DEBU[2023/05/05 17:02:43] [smtp] not allowed to relay to example@gmail.com: does not match the allowlist @example\.com$ 
DEBU[2023/05/05 17:02:43] [smtp] relayed message from ralph@ralph via mail.local:25

... so I've changed Send() to return an error instead of nil if len(recipients) == 0, and changed the relay code in mailHandler to show a warning if Send() "fails" (rather than an error).

One last question: I'm not sure how to best document the required yaml regex syntax in the documentation (wiki) in a way that users will understand. Do you have any suggestions?

@gliwka
Copy link
Contributor Author

gliwka commented May 5, 2023

@axllent

Thanks for the quick merge :-)

Good that you caught the bug on sending without recipients.

Documentation wise I would make an example using a literal-style YAML string checking for a single hostname, since this will be a common use case. The literal string will make it easier, since you don’t have to escape backslashes.

recipient-whitelist: '@example\.org$'

You can link to this regex playground making it easier for users to find the right one: https://regex101.com/r/mfbicW/1

In which release will this feature land? What’s your release cadence?

@axllent
Copy link
Owner

axllent commented May 5, 2023

Thanks for the documentation suggestions, and the tip about linking to regex101 is a good one (I hadn't thought of that).

As for releases - this should be in the next release which will probably happen within the next 24-48 hours. I prefer to release often (when there are changes of course) rather than build up lots of changes. Mailpit was more-or-less feature-complete for my needs about 6 months ago, but I have been adding more and more features for other users like yourself, so most releases add these new small features :-)

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.

2 participants