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

feat: Naive attempt to add SES role based auth for emails, instead of SMTP. #434

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

cafuego
Copy link
Contributor

@cafuego cafuego commented Apr 12, 2024

Perhaps this should be in config, and pass a single mail transport back to the module.

Refs: OPS-10025

cafuego and others added 3 commits April 12, 2024 14:58
… SMTP.

Perhaps this should be in config, and pass a single mail transport back to the module.

Refs: OPS-10025
There were install conflicts using the newer, stricter
peer deps logic, but using legacy allows the install to
finish successfully.

Refs: OPS-10025
@rupl
Copy link
Contributor

rupl commented Apr 12, 2024

I'd tend to agree it should be in config, but I also don't have so strong of an opinion. I added the deps and linted for you. Should we deploy an image of this to dev and see if it works? Is it even possible to test when mailhog is scooping up emails?

@cafuego
Copy link
Contributor Author

cafuego commented Apr 15, 2024

Sadly, no. There is no way to test this change with mailhog. When mail is handled via SES api calls, the data is handed off to an AWS service endpoint and the only thing we know is the API response code and the (eventual) log in ELK. There is no way to intercept that.

To make things worse, the there is no test endpoint to do a dry run with; to check whether or no the API calls work or fail there are magic destination email address that will trigger a success or fail. Any other email addresses will cause an actual email to be delivered.

That means we cannot test this with an actual HID database; at best we can spin up an env with only our own accounts in it and make it send mail.

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