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

Add support for Postmark bounce notifications via webhooks #1385

Closed
wants to merge 7 commits into from

Conversation

tosie
Copy link
Contributor

@tosie tosie commented Jun 30, 2023

Please excuse me from not posting a feature issue first. The implementation was not that much work and I felt it would be easier to just write code and talk about that.

This PR adds support for Postmark's bounce notifications.

Currently, Listmonk has support for SES and Sendgrid bounce notifications. I use Postmark and would like to use Listmonk, so it made sense to contribute something to this project that might be beneficial for other users, too.

There are two main areas of this PR:

  1. Adding a setting to enable/disable the Postmark webhook service (including translations in the frontend).
  2. Implementing a Postmark webhook service to handle bounce notifications.

There is also a migration (v2.6.0) that adds the setting key to the DB.

@tosie tosie marked this pull request as ready for review June 30, 2023 16:44
@knadh
Copy link
Owner

knadh commented Jul 6, 2023

Thank you for the PR, @tosie. I'll review this in the next couple of weeks for the upcoming release.

@knadh knadh added the enhancement New feature or request label Jul 6, 2023
@knadh
Copy link
Owner

knadh commented Jul 12, 2023

PR looks good @tosie. One minor correction. The upcoming version is v2.5.0. Please remove the v2.6.0 migration and add it to v2.5.0 instead.

@tosie
Copy link
Contributor Author

tosie commented Jul 12, 2023

PR looks good @tosie. One minor correction. The upcoming version is v2.5.0. Please remove the v2.6.0 migration and add it to v2.5.0 instead.

Done ✅

@knadh
Copy link
Owner

knadh commented Jul 20, 2023

Thanks! Tested this. Was reviewing the Postmark docs before merging and realised that it supports HTTP BasicAuth for webhooks. Without this, the current implementation would be open to abuse (unauthenticated POST).

@tosie
Copy link
Contributor Author

tosie commented Jul 20, 2023

You are right. I would try to add that, but might need help with the translations …

@knadh
Copy link
Owner

knadh commented Jul 27, 2023

Hi @tosie. I've been experimenting with GPT-4 auto-translating missing strings in language packs using English as the base. The results are pretty good. Will finish this script and bundle in the repo soon. You can add just the English strings and leave it.

@tosie
Copy link
Contributor Author

tosie commented Aug 18, 2023

This PR is getting big, @knadh :-). Basic auth for Postmark webhooks is implemented now. You can take another look, if you'd like to.

@tosie
Copy link
Contributor Author

tosie commented Aug 20, 2023

Rebased on current master.

@knadh
Copy link
Owner

knadh commented Aug 27, 2023

I'd made a few changes to this PR last week. Only got time to finish it just now. Could you please take a look? 0289c01

@tosie
Copy link
Contributor Author

tosie commented Aug 28, 2023

Your commit removes the HTTP basic auth handler completely. I'll try to re-add that to the bounce package (inside of the postmark handler code).

@knadh
Copy link
Owner

knadh commented Aug 28, 2023

Ah, my bad. Added it here: a96b243

@tosie
Copy link
Contributor Author

tosie commented Aug 28, 2023

Looks good 👍. Thanks!

@tosie
Copy link
Contributor Author

tosie commented Aug 28, 2023

Closed in favor of #1485.

@tosie tosie closed this Aug 28, 2023
knadh added a commit that referenced this pull request Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants