rcpt_ok: refactored and added tests

plugins/rcpt_ok

@@ -8,41 +8,92 @@ rcpt_ok
 
 this plugin checks the standard rcpthosts config
 
+=head1 DESCRIPTION
+
+Check the recipient hostname and determine if we accept mail to that host.
+
+This is functionally identical to qmail's rcpthosts implementation, consulting
+both rcpthosts and morercpthosts.cdb.
+
+=head1 CONFIGURATION
+
 It should be configured to be run _LAST_!
 
 =cut
 
+use strict;
+use warnings;
+
+use Qpsmtpd::Constants;
 use Qpsmtpd::DSN;
 
 sub hook_rcpt {
   my ($self, $transaction, $recipient, %param) = @_;
-  my $host = lc $recipient->host;
-
-  my @rcpt_hosts = ($self->qp->config("me"), $self->qp->config("rcpthosts"));
 
   # Allow 'no @' addresses for 'postmaster' and 'abuse'
   # qmail-smtpd will do this for all users without a domain, but we'll
   # be a bit more picky.  Maybe that's a bad idea.
-  my $user = $recipient->user;
+    my $host = $self->get_rcpt_host( $recipient ) or return (OK);
-  $host = $self->qp->config("me")
+
-    if ($host eq "" && (lc $user eq "postmaster" || lc $user eq "abuse"));
+    return (OK) if $self->is_in_rcpthosts( $host );
+    return (OK) if $self->is_in_morercpthosts( $host );
+    return (OK) if $self->qp->connection->relay_client;  # failsafe
+
+    # default of relaying_denied is obviously DENY,
+    # we use the default "Relaying denied" message...
+    return Qpsmtpd::DSN->relaying_denied();
+}
+
+sub is_in_rcpthosts {
+    my ( $self, $host ) = @_;
+
+    my @rcpt_hosts = ($self->qp->config('me'), $self->qp->config('rcpthosts'));
 
     # Check if this recipient host is allowed
     for my $allowed (@rcpt_hosts) {
         $allowed =~ s/^\s*(\S+)/$1/;
-    return (OK) if $host eq lc $allowed;
+        if ( $host eq lc $allowed ) {
-    return (OK) if substr($allowed,0,1) eq "." and $host =~ m/\Q$allowed\E$/i;
+            $self->log( LOGINFO, "pass: $host in rcpthosts" );
+            return 1;
+        };
+
+        if ( substr($allowed,0,1) eq '.' and $host =~ m/\Q$allowed\E$/i ) {
+            $self->log( LOGINFO, "pass: $host in rcpthosts as $allowed" );
+            return 1;
+        };
     }
 
+    return;
+};
+
+sub is_in_morercpthosts {
+    my ( $self, $host ) = @_;
+
     my $more_rcpt_hosts = $self->qp->config('morercpthosts', 'map');
-  return (OK) if exists $more_rcpt_hosts->{$host};
 
-  if ( $self->qp->connection->relay_client ) { # failsafe
+    if ( exists $more_rcpt_hosts->{$host} ) {
-    return (OK);
+        $self->log( LOGINFO, "pass: $host found in morercpthosts" );
-  }
+        return 1;
-  else {
+    };
-    # default of relaying_denied is obviously DENY, 
+
-    # we use the default "Relaying denied" message...
+    $self->log( LOGINFO, "fail: $host not in morercpthosts" );
-    return Qpsmtpd::DSN->relaying_denied();
+    return;
-  }
+};
-}
+
+sub get_rcpt_host {
+    my ( $self, $recipient ) = @_;
+
+    return if ! $recipient;  # Qpsmtpd::Address couldn't parse the recipient
+
+    if ( $recipient->host ) {
+        return lc $recipient->host;
+    };
+
+    # no host portion exists
+    my $user = $recipient->user or return;
+    if ( lc $user eq 'postmaster' || lc $user eq 'abuse' ) {
+        return $self->qp->config('me');
+    };
+    return;
+};
+

t/plugin_tests/rcpt_ok

@@ -1,22 +1,98 @@
+#!perl -w
+
+use strict;
+use warnings;
+
+use Qpsmtpd::Constants;
 
 sub register_tests {
     my $self = shift;
-    $self->register_test("test_returnval", 2);
+
-    $self->register_test("rcpt_ok", 1);
+    $self->register_test('test_get_rcpt_host', 7);
+    $self->register_test('test_is_in_rcpthosts', 3);
+    $self->register_test('test_is_in_morercpthosts', 2);
+    $self->register_test('test_hook_rcpt', 3);
 }
 
-sub test_returnval {
+
+sub test_hook_rcpt {
     my $self = shift;
-    my $address = Qpsmtpd::Address->parse('<me@example.com>');
-    my ($ret, $note) = $self->hook_rcpt($self->qp->transaction, $address);
-    is($ret, DENY, "Check we got a DENY");
-    print("# rcpt_ok result: $note\n");
-    $address = Qpsmtpd::Address->parse('<me@localhost>');
-    ($ret, $note) = $self->hook_rcpt($self->qp->transaction, $address);
-    is($ret, OK, "Check we got a OK");
-#    print("# rcpt_ok result: $note\n");
-}
 
-sub rcpt_ok {
+    my $transaction = $self->qp->transaction;
-    ok(1);
+
-}
+    my $address = Qpsmtpd::Address->parse('<user@localhost>');
+    my ($r, $mess) = $self->hook_rcpt( $transaction, $address );
+    cmp_ok( $r, '==', OK, "hook_rcpt, localhost");
+
+    $address = Qpsmtpd::Address->parse('<user@example.com>');
+    ($r, $mess) = $self->hook_rcpt( $transaction, $address );
+    cmp_ok( $r, '==', DENY, "hook_rcpt, example.com");
+
+    $self->qp->connection->relay_client(1);
+    ($r, $mess) = $self->hook_rcpt( $transaction, $address );
+    cmp_ok( $r, '==', OK, "hook_rcpt, example.com");
+    $self->qp->connection->relay_client(0);
+};
+
+sub test_is_in_rcpthosts {
+    my $self = shift;
+
+    my @hosts = $self->qp->config('rcpthosts');
+    my $host = $hosts[0];
+
+    if ( $host ) {
+        ok( $self->is_in_rcpthosts( $host ), "is_in_rcpthosts, $host");
+    }
+    else {
+        ok(1, "is_in_rcpthosts (skip, no entries)" );
+    };
+
+    ok( $self->is_in_rcpthosts( 'localhost' ), "is_in_rcpthosts +");
+    ok( ! $self->is_in_rcpthosts( 'example.com' ), "is_in_rcpthosts -");
+};
+
+sub test_is_in_morercpthosts {
+    my $self = shift;
+
+    my $ref = $self->qp->config('morercpthosts', 'map');
+    my ($domain) = keys %$ref;
+    if ( $domain ) {
+        ok( $self->is_in_morercpthosts( $domain ), "is_in_morercpthosts, $domain");
+    }
+    else {
+        ok(1, "is_in_morercpthosts (skip, no entries)" );
+    };
+
+    ok( ! $self->is_in_morercpthosts( 'example.com' ), "is_in_morercpthosts -");
+};
+
+sub test_get_rcpt_host {
+    my $self = shift;
+
+    my $address = Qpsmtpd::Address->parse('<me@example.com>');
+    cmp_ok( $self->get_rcpt_host( $address ), 'eq', 'example.com', 
+        "get_rcpt_host, +" );
+    
+    $address = Qpsmtpd::Address->parse('<me@exaMple.com>');
+    cmp_ok( $self->get_rcpt_host( $address ), 'eq', 'example.com', 
+        "get_rcpt_host, +" );
+
+    $address = Qpsmtpd::Address->parse('<root@example.com>');
+    cmp_ok( $self->get_rcpt_host( $address ), 'eq', 'example.com', 
+        "get_rcpt_host, +" );
+
+    $address = Qpsmtpd::Address->parse('<postmaster>');
+    cmp_ok( $self->get_rcpt_host( $address ), 'eq', 'some.host.example.org',
+        "get_rcpt_host, special postmaster +" );
+
+    # I think this is a bug. Qpsmtpd::Address fails to parse <abuse>
+    $address = Qpsmtpd::Address->parse('<abuse>');
+    ok( ! $self->get_rcpt_host( $address ), "get_rcpt_host, missing host" );
+
+    $address = Qpsmtpd::Address->parse('<>');
+    ok( ! $self->get_rcpt_host( $address ), "get_rcpt_host, null recipient" );
+
+    $address = Qpsmtpd::Address->parse('<@example.com>');
+    ok( ! $self->get_rcpt_host( $address ), "get_rcpt_host, missing user" );
+};
+