haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.02k stars 662 forks source link

Bounce hook rethink #454

Closed baudehlo closed 10 years ago

baudehlo commented 10 years ago

This is mostly for Steve's benefit:

I fixed the bug in outbound temp failing, but it left me thinking about how we currently pass in bounce_extra in a very bizarre way... Currently we pass in something like this:

[ {"matt@sergeant.org": "Some reason"}, {"bob@domain": "Other reason"} ]

It's the weirdest data structure ever.

I propose we pass in a list of Address objects that we augment with a .reason parameter.

Only problem is if people are using bounce hooks and looking at bounce_extra. I suspect you (Steve) might be the only one because anyone else would complain about that odd data structure. If they are, we could use a different key such as hmail.bounce_users and keep generating bounce_extras the weird way it is constructed.

Thoughts?

smfreegard commented 10 years ago

No one else is using this as it only went out in 2.3.0 didn't it, so there's no need to maintain the weird structure.

I was using it in a plugin to track bounces. I added it because we were losing the actual response from the upstream SMTP server so didn't provide the necessary detail.

var outbound = require('./outbound');

exports.hook_bounce = function (next, hmail, err) {
    var todo = hmail.todo;
    if (!todo) return next();

    var cfg = this.config.get('alerts.ini');
    var alerts_to = (cfg.main.alerts_to) ? cfg.main.alerts_to.toString().replace(/\s+/g,' ').split(/[;, ]+/) : undefined;
    var alerts_from = cfg.main.alerts_from || 'postmaster@' + this.config.get('me');

    // TODO: handle alert actions
    // e.g. disable account, quarantine, etc.

    if (alerts_to && alerts_to.length) {
        var contents = [
            'From: "MAILER DAEMON" <' + alerts_from + '>',
            'To: <' + alerts_to.join('>, <') + '>',
            'Subject: Administrative Alert',
            'MIME-Version: 1.0',
            'Content-Type: text/plain; charset=utf-8',
            '',
            'Date/Time   : ' + (new Date).toUTCString(),
            'ID          : ' + todo.uuid,
            'Host/IP     : ' + ((todo.notes.remote_host) ? todo.notes.remote_host + ' ': '') + '[' + todo.notes.remote_ip + ']',
            'Auth User:  : ' + ((todo.notes.auth_user) ? todo.notes.auth_user : ''),
            'Sender      : ' + ((todo && todo.mail_from) ? todo.mail_from.original : ''),
            'Recipient(s): ' + todo.rcpt_to.map(function (recip) { return recip.original }).join(', '),
            'Detail      : ' + err,
        ].join("\n");
        if (hmail.bounce_extra) {
            contents += '\n\n' + hmail.bounce_extra.map(function (item) {
                var recip = Object.keys(item)[0];
                return recip + ': ' + item[recip];
            }).join('\n');
        }

        // Send message
        outbound.send_email(alerts_from, alerts_to, contents);
    }
    return next();
}

Output from this yielded:

Date/Time   : Tue, 11 Feb 2014 09:43:27 GMT
ID          : D04627DD-AC2D-48D5-BC56-0857E1E7485A.1
Host/IP     : y01acxsmtp001.ahe.pok.ibm.com [129.33.206.182]
Auth User:  : 
Sender      : <mktcmpid@us.ibm.com>
Recipient(s): <r_box_5@fsl.com>
Detail      : Some recipients failed: 

<r_box_5@fsl.com>: 550 5.7.1 <r_box_5@fsl.com>: Relay access denied.

Adding .reason to the Address object is a far better way to do this and we can get rid of the bounce_extra structure entirely which is much neater.

I've applied your changes, but I'm not seeing .reason being populated:

Adding:

contents += '\n\n' + JSON.stringify(todo.rcpt_to);

To the message to look at the rcpt_to structure yields:

Date/Time   : Tue, 11 Feb 2014 10:50:18 GMT
ID          : B447DCDE-66D9-4F4F-B4F0-475D3A898203.1
Host/IP     : [undefined]
Auth User:  : 
Sender      : <steve.freegard@fsl.com>
Recipient(s): <sldfjslkjsdfsdf@stevefreegard.com>
Detail      : Some recipients failed: <sldfjslkjsdfsdf@stevefreegard.com>

[{"original":"<sldfjslkjsdfsdf@stevefreegard.com>","user":"sldfjslkjsdfsdf","host":"stevefreegard.com"}]
baudehlo commented 10 years ago

Typo fixed.

On Tue, Feb 11, 2014 at 6:00 AM, Steve Freegard notifications@github.comwrote:

No one else is using this as it only went out in 2.3.0 didn't it, so there's no need to maintain the weird structure.

I was using it in a plugin to track bounces. I added it because we were losing the actual response from the upstream SMTP server so didn't provide the necessary detail.

var outbound = require('./outbound'); exports.hook_bounce = function (next, hmail, err) { var todo = hmail.todo; if (!todo) return next();

var cfg = this.config.get('alerts.ini');
var alerts_to = (cfg.main.alerts_to) ? cfg.main.alerts_to.toString().replace(/\s+/g,' ').split(/[;, ]+/) : undefined;
var alerts_from = cfg.main.alerts_from || 'postmaster@' + this.config.get('me');

// TODO: handle alert actions
// e.g. disable account, quarantine, etc.

if (alerts_to && alerts_to.length) {
    var contents = [
        'From: "MAILER DAEMON" <' + alerts_from + '>',
        'To: <' + alerts_to.join('>, <') + '>',
        'Subject: Administrative Alert',
        'MIME-Version: 1.0',
        'Content-Type: text/plain; charset=utf-8',
        '',
        'Date/Time   : ' + (new Date).toUTCString(),
        'ID          : ' + todo.uuid,
        'Host/IP     : ' + ((todo.notes.remote_host) ? todo.notes.remote_host + ' ': '') + '[' + todo.notes.remote_ip + ']',
        'Auth User:  : ' + ((todo.notes.auth_user) ? todo.notes.auth_user : ''),
        'Sender      : ' + ((todo && todo.mail_from) ? todo.mail_from.original : ''),
        'Recipient(s): ' + todo.rcpt_to.map(function (recip) { return recip.original }).join(', '),
        'Detail      : ' + err,
    ].join("\n");
    if (hmail.bounce_extra) {
        contents += '\n\n' + hmail.bounce_extra.map(function (item) {
            var recip = Object.keys(item)[0];
            return recip + ': ' + item[recip];
        }).join('\n');
    }

    // Send message
    outbound.send_email(alerts_from, alerts_to, contents);
    return next();}

Output from this yielded:

Date/Time : Tue, 11 Feb 2014 09:43:27 GMT ID : D04627DD-AC2D-48D5-BC56-0857E1E7485A.1 Host/IP : y01acxsmtp001.ahe.pok.ibm.com [129.33.206.182] Auth User: : Sender : mktcmpid@us.ibm.com Recipient(s): r_box_5@fsl.com Detail : Some recipients failed:

r_box_5@fsl.com: 550 5.7.1 r_box_5@fsl.com: Relay access denied.

Adding .reason to the Address object is a far better way to do this and we can get rid of the bounce_extra structure entirely which is much neater.

I've applied your changes, but I'm not seeing .reason being populated:

Adding:

contents += '\n\n' + JSON.stringify(todo.rcpt_to);

To the message to look at the rcpt_to structure yields:

Date/Time : Tue, 11 Feb 2014 10:50:18 GMT ID : B447DCDE-66D9-4F4F-B4F0-475D3A898203.1 Host/IP : [undefined] Auth User: : Sender : steve.freegard@fsl.com Recipient(s): sldfjslkjsdfsdf@stevefreegard.com Detail : Some recipients failed: sldfjslkjsdfsdf@stevefreegard.com

[{"original":"sldfjslkjsdfsdf@stevefreegard.com","user":"sldfjslkjsdfsdf","host":"stevefreegard.com"}]

— Reply to this email directly or view it on GitHubhttps://github.com/baudehlo/Haraka/issues/454#issuecomment-34744177 .