[Rt-commit] rt branch, 4.4/multiple-reply-to, created. rt-4.2.3-73-g395e0ca

? sunnavy sunnavy at bestpractical.com
Wed Nov 5 07:18:56 EST 2014


The branch, 4.4/multiple-reply-to has been created
        at  395e0ca730b634f50c8227576fa1620aafbb8eb2 (commit)

- Log -----------------------------------------------------------------
commit bb657e933520dab5f5b621377a55731c64614120
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 2 22:59:45 2012 +0400

    simple test for multiple reply-to addresses

diff --git a/t/mail/multiple-reply-to.t b/t/mail/multiple-reply-to.t
new file mode 100644
index 0000000..2afa96c
--- /dev/null
+++ b/t/mail/multiple-reply-to.t
@@ -0,0 +1,51 @@
+use strict;
+use warnings;
+
+use RT::Test tests => 7;
+
+diag "grant everybody with CreateTicket right";
+{
+    ok( RT::Test->set_rights(
+        { Principal => 'Everyone', Right => [qw(CreateTicket)], },
+        { Principal => 'Requestor', Right => [qw(ReplyToTicket)], },
+    ), "Granted rights");
+}
+
+{
+    my $text = <<EOF;
+From: user\@example.com
+Subject: test
+
+Blah!
+Foob!
+EOF
+    my ($status, $id) = RT::Test->send_via_mailgate($text);
+    is ($status >> 8, 0, "The mail gateway exited normally");
+    ok ($id, "ticket created");
+
+    $text = <<EOF;
+From: user\@example.com
+Subject: [@{[RT->Config->Get('rtname')]} #$id] test
+
+Blah!
+Foob!
+EOF
+    ($status, my $tid) = RT::Test->send_via_mailgate($text);
+    is ($status >> 8, 0, "The mail gateway exited normally");
+    is ($tid, $id, "ticket updated");
+
+    $text = <<EOF;
+From: somebody\@example.com
+Reply-To: boo\@example.com, user\@example.com
+Subject: [@{[RT->Config->Get('rtname')]} #$id] test
+
+Blah!
+Foob!
+EOF
+    ($status, $tid) = RT::Test->send_via_mailgate($text);
+    is ($status >> 8, 0, "The mail gateway exited normally");
+    is ($tid, $id, "ticket updated");
+}
+
+
+1;

commit 395e0ca730b634f50c8227576fa1620aafbb8eb2
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Oct 7 00:01:11 2014 +0800

    take into account multiple sender's addresses
    
    use first address that has rights, if none has rights then do what we did
    before.

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 9fc60cc..5d71f65 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -219,7 +219,7 @@ sub Gateway {
     TMPFAIL("RT couldn't find the queue: " . $args{'queue'})
         unless $SystemTicket->id || $SystemQueueObj->id;
 
-    my $CurrentUser = GetCurrentUser(
+    my @CurrentUsers = GetCurrentUser(
         Message       => $Message,
         RawMessageRef => \$args{message},
         Ticket        => $SystemTicket,
@@ -228,10 +228,10 @@ sub Gateway {
 
     # We only care about ACLs on the _first_ action, as later actions
     # may have gotten rights by the time they happen.
-    CheckACL(
+    my $CurrentUser = CheckACL(
         Action        => $actions[0],
         Message       => $Message,
-        CurrentUser   => $CurrentUser,
+        CurrentUser  => \@CurrentUsers,
         Ticket        => $SystemTicket,
         Queue         => $SystemQueueObj,
     );
@@ -306,15 +306,15 @@ sub Plugins {
 
 Dispatches to the C<@MailPlugins> to find one the provides
 C<GetCurrentUser> that recognizes the current user.  Mail plugins are
-tried one at a time, and stops after the first to return a current user.
+tried one at a time, and stops after the first to return at least one current user.
 Anonymous subroutine references found in C<@MailPlugins> are treated as
 C<GetCurrentUser> methods.
 
 The default GetCurrentUser authenticator simply looks at the From:
-address, and loads or creates a user accordingly; see
+address, and loads or creates users accordingly; see
 L<RT::Interface::Email::Auth::MailFrom>.
 
-Returns the current user; on failure of any plugin to do so, stops
+Returns a list of possible CurrentUsers; on failure of any plugin to do so, stops
 processing with a permanent failure and sends a generic "Permission
 Denied" mail to the user.
 
@@ -331,13 +331,13 @@ sub GetCurrentUser {
 
     # Since this needs loading, no matter what
     for my $Code ( Plugins(Code => 1, Method => "GetCurrentUser") ) {
-        my $CurrentUser = $Code->(
+        my @CurrentUsers = $Code->(
             Message       => $args{Message},
             RawMessageRef => $args{RawMessageRef},
             Ticket        => $args{Ticket},
             Queue         => $args{Queue},
         );
-        return $CurrentUser if $CurrentUser and $CurrentUser->id;
+        return @CurrentUsers if @CurrentUsers;
     }
 
     # None of the GetCurrentUser plugins found a user.  This is
@@ -350,9 +350,9 @@ sub GetCurrentUser {
     );
 }
 
-=head3 CheckACL Action => C<action>, CurrentUser => C<user>, Ticket => C<ticket>, Queue => C<queue>
+=head3 CheckACL Action => C<action>, CurrentUser => [C<user>] or C<user>, Ticket => C<ticket>, Queue => C<queue>
 
-Checks that the currentuser can perform a particular action.  While RT's
+Checks that the currentusers can perform a particular action.  While RT's
 standard permission controls apply, this allows a better error message,
 or more limited restrictions on the email gateway.
 
@@ -364,6 +364,8 @@ and abort with failure of their own accord.
 Aborts processing, sending a "Permission Denied" mail to the user with
 the last plugin's failure message, on failure.
 
+Returns the first user that passed the check.
+
 =cut
 
 sub CheckACL {
@@ -377,13 +379,14 @@ sub CheckACL {
     );
 
     for my $Code ( Plugins( Method => "CheckACL" ) ) {
-        return if $Code->(
-            Message       => $args{Message},
-            CurrentUser   => $args{CurrentUser},
-            Action        => $args{Action},
-            Ticket        => $args{Ticket},
-            Queue         => $args{Queue},
+        my $CurrentUser = $Code->(
+            Message     => $args{Message},
+            CurrentUser => $args{CurrentUser},
+            Action      => $args{Action},
+            Ticket      => $args{Ticket},
+            Queue       => $args{Queue},
         );
+        return $CurrentUser if $CurrentUser;
     }
 
     # Nobody said yes, and nobody said FAILURE; fail closed
@@ -435,19 +438,39 @@ investigate the parse failure.
 =cut
 
 sub ParseSenderAddressFromHead {
+    my ( $list, @errors ) = ParseSenderAddressesFromHead(@_);
+    if ( $list ) {
+        return ( $list->[0]->address, $list->[0]->phrase, @errors );
+    }
+    else {
+        return ( undef, undef, @errors );
+    }
+}
+
+=head2 ParseSenderAddressesFromHead HEAD
+
+Takes a MIME::Header object. Returns ([list of addreses], errors)
+where the addresses are found in C<Reply-To>, C<From> and C<Sender>.
+
+=cut
+
+sub ParseSenderAddressesFromHead {
     my $head = shift;
     my @errors;  # Accumulate any errors
 
+    my @addr;
     foreach my $header ( 'Reply-To', 'From', 'Sender' ) {
         my $addr_line = $head->get($header) || next;
-        my ($addr) = RT::EmailParser->ParseEmailAddress( $addr_line );
-        return ($addr->address, $addr->phrase, @errors) if $addr;
+        push @addr, RT::EmailParser->ParseEmailAddress( $addr_line );
 
-        chomp $addr_line;
-        push @errors, "$header: $addr_line";
+        unless ( @addr ) {
+            chomp $addr_line;
+            push @errors, "$header: $addr_line";
+        }
     }
 
-    return (undef, undef, @errors);
+    return (\@addr, @errors) if @addr;
+    return (undef, @errors);
 }
 
 =head3 ParseErrorsToAddressFromHead HEAD
diff --git a/lib/RT/Interface/Email/Action/Resolve.pm b/lib/RT/Interface/Email/Action/Resolve.pm
index 452a01b..fb124d2 100644
--- a/lib/RT/Interface/Email/Action/Resolve.pm
+++ b/lib/RT/Interface/Email/Action/Resolve.pm
@@ -89,15 +89,17 @@ sub CheckACL {
         );
     }
 
-    my $principal = $args{CurrentUser}->PrincipalObj;
-    return 1 if $principal->HasRight( Object => $args{'Ticket'}, Right  => 'ModifyTicket' );
-
-    my $email = $args{CurrentUser}->UserObj->EmailAddress;
+    my @emails;
+    for my $CurrentUser ( ref $args{CurrentUser} eq 'ARRAY' ? @{ $args{CurrentUser} } : $args{CurrentUser} ) {
+        my $principal = $CurrentUser->PrincipalObj;
+        return $CurrentUser if $principal->HasRight( Object => $args{'Ticket'}, Right => 'ModifyTicket' );
+        push @emails, $CurrentUser->UserObj->EmailAddress;
+    }
     my $qname = $args{Queue}->Name;
     my $tid   = $args{Ticket}->id;
     MailError(
         Subject     => "Permission Denied",
-        Explanation => "$email has no right to own ticket $tid in queue $qname",
+        Explanation => (@emails == 1 ? "@emails has" : join(", ", @emails) . " have") . " no right to own ticket $tid in queue $qname",
         FAILURE     => 1,
     );
 }
diff --git a/lib/RT/Interface/Email/Action/Take.pm b/lib/RT/Interface/Email/Action/Take.pm
index f7e2191..286a84b 100644
--- a/lib/RT/Interface/Email/Action/Take.pm
+++ b/lib/RT/Interface/Email/Action/Take.pm
@@ -89,15 +89,18 @@ sub CheckACL {
         );
     }
 
-    my $principal = $args{CurrentUser}->PrincipalObj;
-    return 1 if $principal->HasRight( Object => $args{'Ticket'}, Right  => 'OwnTicket' );
+    my @emails;
+    for my $CurrentUser ( ref $args{CurrentUser} eq 'ARRAY' ? @{ $args{CurrentUser} } : $args{CurrentUser} ) {
+        my $principal = $CurrentUser->PrincipalObj;
+        return $CurrentUser if $principal->HasRight( Object => $args{'Ticket'}, Right  => 'OwnTicket' );
+        push @emails, $CurrentUser->UserObj->EmailAddress;
+    }
 
-    my $email = $args{CurrentUser}->UserObj->EmailAddress;
     my $qname = $args{Queue}->Name;
     my $tid   = $args{Ticket}->id;
     MailError(
         Subject     => "Permission Denied",
-        Explanation => "$email has no right to own ticket $tid in queue $qname",
+        Explanation => (@emails == 1 ? "@emails has" : join(", ", @emails) . " have") . " no right to own ticket $tid in queue $qname",
         FAILURE     => 1,
     );
 }
diff --git a/lib/RT/Interface/Email/Auth/MailFrom.pm b/lib/RT/Interface/Email/Auth/MailFrom.pm
index 081a570..e97f3a2 100644
--- a/lib/RT/Interface/Email/Auth/MailFrom.pm
+++ b/lib/RT/Interface/Email/Auth/MailFrom.pm
@@ -64,10 +64,9 @@ This is the default authentication plugin for RT's email gateway; no no
 other authentication plugin is found in L<RT_Config/@MailPlugins>, RT
 will default to this one.
 
-This plugin reads the first address found in the C<Reply-To>, C<From>,
-and C<Sender> headers, and loads or creates the user.  It performs no
-checking of the identity of the user, and trusts the headers of the
-incoming email.
+This plugin reads addresses found in the C<Reply-To>, C<From>, and C<Sender>
+headers, and loads or creates users accordingly. It performs no checking of
+the identity of the users, and trusts the headers of the incoming email.
 
 =cut
 
@@ -78,35 +77,35 @@ sub GetCurrentUser {
     );
 
     # We don't need to do any external lookups
-    my ( $Address, $Name, @errors ) = RT::Interface::Email::ParseSenderAddressFromHead( $args{'Message'}->head );
-    $RT::Logger->warning("Failed to parse ".join(', ', @errors))
-        if @errors;
-
-    unless ( $Address ) {
+    my ($addresses, @errors) = RT::Interface::Email::ParseSenderAddressesFromHead( $args{'Message'}->head );
+    $RT::Logger->warning("Failed to parse ".join(', ', @errors)) if @errors;
+    unless ( $addresses ) {
         $RT::Logger->error("Couldn't parse or find sender's address");
         FAILURE("Couldn't parse or find sender's address");
     }
 
-    my $CurrentUser = RT::CurrentUser->new;
-    $CurrentUser->LoadByEmail( $Address );
-    $CurrentUser->LoadByName( $Address ) unless $CurrentUser->Id;
-    if ( $CurrentUser->Id ) {
-        $RT::Logger->debug("Mail from user #". $CurrentUser->Id ." ($Address)" );
-        return $CurrentUser;
+    my @CurrentUsers;
+    foreach my $addr ( @$addresses ) {
+        $RT::Logger->debug("Testing $addr as sender");
+
+        my $CurrentUser = RT::CurrentUser->new;
+        $CurrentUser->LoadByEmail( $addr->address );
+        $CurrentUser->LoadByName( $addr->address ) unless $CurrentUser->Id;
+        unless ( $CurrentUser->Id ) {
+            my $user = RT::User->new( RT->SystemUser );
+            $user->LoadOrCreateByEmail(
+                RealName     => $addr->phrase,
+                EmailAddress => $addr->address,
+                Comments     => 'Autocreated on ticket submission',
+            );
+
+            $CurrentUser = RT::CurrentUser->new;
+            $CurrentUser->Load( $user->id );
+        }
+        push @CurrentUsers, $CurrentUser;
     }
 
-
-    my $user = RT::User->new( RT->SystemUser );
-    $user->LoadOrCreateByEmail(
-        RealName     => $Name,
-        EmailAddress => $Address,
-        Comments     => 'Autocreated on ticket submission',
-    );
-
-    $CurrentUser = RT::CurrentUser->new;
-    $CurrentUser->Load( $user->id );
-
-    return $CurrentUser;
+    return @CurrentUsers;
 }
 
 1;
diff --git a/lib/RT/Interface/Email/Authz/Default.pm b/lib/RT/Interface/Email/Authz/Default.pm
index 9a96a0a..2e558ff 100644
--- a/lib/RT/Interface/Email/Authz/Default.pm
+++ b/lib/RT/Interface/Email/Authz/Default.pm
@@ -78,21 +78,45 @@ sub CheckACL {
         @_,
     );
 
-    my $principal = $args{CurrentUser}->PrincipalObj;
-    my $email     = $args{CurrentUser}->UserObj->EmailAddress;
-    my $qname     = $args{'Queue'}->Name;
+    my @emails;
+    for my $CurrentUser ( ref $args{CurrentUser} eq 'ARRAY' ? @{ $args{CurrentUser} } : $args{CurrentUser} ) {
+        my $principal = $CurrentUser->PrincipalObj;
+        push @emails, $CurrentUser->UserObj->EmailAddress;
+
+        if ( $args{'Ticket'} && $args{'Ticket'}->Id ) {
+            if ( $args{'Action'} =~ /^comment$/i ) {
+                return $CurrentUser if $principal->HasRight( Object => $args{'Ticket'}, Right => 'CommentOnTicket' );
+            }
+            elsif ( $args{'Action'} =~ /^correspond$/i ) {
+                return $CurrentUser if $principal->HasRight( Object => $args{'Ticket'}, Right => 'ReplyToTicket' );
+            }
+            else {
+                $RT::Logger->warning("Action '". ($args{'Action'}||'') ."' is unknown");
+                return;
+            }
+        }
+
+        # We're creating a ticket
+        elsif ( $args{'Action'} =~ /^(comment|correspond)$/i ) {
+            return $CurrentUser if $principal->HasRight( Object => $args{'Queue'}, Right => 'CreateTicket' );
+        }
+        else {
+            $RT::Logger->warning( "Action '" . ( $args{'Action'} || '' ) . "' is unknown with no ticket" );
+            return;
+        }
+    }
 
     my $msg;
+    my $email = join ", ", @emails;
+    my $qname = $args{'Queue'}->Name;
     if ( $args{'Ticket'} && $args{'Ticket'}->Id ) {
-        my $tid   = $args{'Ticket'}->id;
+        my $tid = $args{'Ticket'}->id;
 
         if ( $args{'Action'} =~ /^comment$/i ) {
-            return 1 if $principal->HasRight( Object => $args{'Ticket'}, Right => 'CommentOnTicket' );
-            $msg = "$email has no right to comment on ticket $tid in queue $qname";
+            $msg = (@emails ? "$email has" : "$email have") . " no right to comment on ticket $tid in queue $qname";
         }
         elsif ( $args{'Action'} =~ /^correspond$/i ) {
-            return 1 if $principal->HasRight( Object => $args{'Ticket'}, Right  => 'ReplyToTicket' );
-            $msg = "$email has no right to reply to ticket $tid in queue $qname";
+            $msg = (@emails ? "$email has" : "$email have") . " no right to reply to ticket $tid in queue $qname";
 
             # Also notify the owner
             MailError(
@@ -104,16 +128,10 @@ might need to grant 'Everyone' the ReplyToTicket right.
 EOT
             );
         }
-        else {
-            $RT::Logger->warning("Action '". ($args{'Action'}||'') ."' is unknown");
-            return;
-        }
     }
-
     # We're creating a ticket
     elsif ( $args{'Action'} =~ /^(comment|correspond)$/i ) {
-        return 1 if $principal->HasRight( Object => $args{'Queue'}, Right  => 'CreateTicket' );
-        $msg = "$email has no right to create tickets in queue $qname";
+        $msg = (@emails ? "$email has" : "$email have") . " no right to create tickets in queue $qname";
 
         # Also notify the owner
         MailError(
@@ -125,11 +143,6 @@ might need to grant 'Everyone' the CreateTicket right.
 EOT
         );
     }
-    else {
-        $RT::Logger->warning("Action '". ($args{'Action'}||'') ."' is unknown with no ticket");
-        return;
-    }
-
 
     MailError(
         Subject     => "Permission Denied",

-----------------------------------------------------------------------


More information about the rt-commit mailing list