[bug] send mail event hook fails with multiple recipients

[signebedi]

In Python 2, you should be able to just use comma separated list represented as a string. https://stackoverflow.com/a/12422921. However, apparently this does not work in Python 3 and we need to use send_message instead, see https://docs.python.org/3/library/email.examples.html.

Originally posted by @signebedi in #313 (comment)

[signebedi]

Actually, this might be an issue in reconstructing the list.

libreforms-fastapi/libreforms_fastapi/utils/smtp.py

Line 77 in e9d71c5

server.sendmail(self.from_address, [to_address]+cc_address_list, msg.as_string())

The code base tries to cast the the To string as a list ... so, we should try to just fix that, since it's embedding the list element into a list such that [[recipients]].

[signebedi]

Add option to obfuscate to_addresses
There are circumstances where we may want to obfuscate the recipients of an email when using send_mail. Doing so would mean sending a single email for each recipient (and possibly also CC recipient). This could be simple from an API perspective: we just add a obfuscate_recipients: bool = False param to send_mail. But, there are a few ways that the complexity could compound; what if we want to obfuscate CC addresses, too?

It goes without saying that enabling this feature will risk higher utilization of SMTP outgoing mail. For managed services like AWS SES, this could significantly increase costs - that is a tradeoff that enterprises should consider before enabling this feature.

[signebedi]

Right now, we are addressing this issue by sending to only one recipient at a time. This increases quota usage, see also #327. And, really, we SHOULD add support for multiple email recipients in a single email...

[signebedi]

Sure enough, after adding some debug helpers, it looks like some outgoing mail providers like Amazon SES assess email addresses, see below. This is because the Amazon SES account in question is in sandbox mode, see https://stackoverflow.com/a/37528929.

An SMTP error occurred while sending to [email protected]: (554, b'Message rejected: Email address is not verified. The following identities failed the check in region US-EAST-1: [email protected]')

In some ways, that's a good sign: it shows that the application logic is not flawed. However, it also suggests that sending mail individually is the superior option, because of the variety of ways in which outgoing mail providers can return errors that this application may not be able to handle.

I will keep the default behavior of this smtp wrapper library to send mail individually. In #237, we propose making this a configurable setting in the app config. If we go this route - or, really, even if we don't - we should log failures. This will require us to pass a log object along to smtp each time we call it. This may cause some potential complexity; in any event, I'll make the logger optional.

[signebedi]

Added the logger and closing this issue. However, there might be log handler issues stemming from too many concurrent instances of Mailer (see comparable problem for doc_db logging when using a dependency injection: #226) ... I think we'll be fine because we are using a single logger instance instantiated at app start up, we should ideally be fine... but, if push comes to shove we can make it only use the logger in development.

[signebedi]

Consider implementing logger singleton
In #226 and possibly also #326 we risk "too many open files" errors because of the way our log handlers work with fastapi dependency injections. A singleton might help... Something like:

def get_logger():
    if 'logger' not in globals():
        global logger

        # This is just some boilerplate ... will need to customize it to utils/logging.py.
        logger = logging.getLogger("my_logger")
        logger.setLevel(logging.DEBUG if config.ENVIRONMENT == "development" else logging.INFO)
        handler = logging.StreamHandler()  # or FileHandler if you want to log to a file
        formatter = logging.Formatter('%(asctime)s - %(name)s - %(levelname)s - %(message)s')
        handler.setFormatter(formatter)
        logger.addHandler(handler)
    return logger

And then pass this as its own dependency in get_mailer:

def get_mailer(logger=Depends(get_logger)):

    # with get_config_context() as config:

    config = get_config_depends()

    # Instantiate the Mailer object
    mailer = Mailer(
        enabled = config.SMTP_ENABLED,
        mail_server = config.SMTP_MAIL_SERVER,
        port = config.SMTP_PORT,
        username = config.SMTP_USERNAME,
        password = config.SMTP_PASSWORD,
        from_address = config.SMTP_FROM_ADDRESS,
        # Cowabunga! We'll see if this causes log handler issues. Added in
        # https://github.com/signebedi/libreforms-fastapi/issues/326.
        logger=logger,
    )

    return mailer