[Rt-commit] rt branch, 4.0/multiple-reply-to, created. rt-4.0.4-283-gd00b098

Ruslan Zakirov ruz at bestpractical.com
Thu Feb 2 15:59:29 EST 2012


The branch, 4.0/multiple-reply-to has been created
        at  d00b0982009e832f1e9701f991dd9153eae8fa63 (commit)

- Log -----------------------------------------------------------------
commit 2770222aed88932472380bd0346d528b40f25e92
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 2 15:28:06 2012 +0400

    extract WhichRightShouldBeChecked function
    
    no functionality changes

diff --git a/lib/RT/Interface/Email/Auth/MailFrom.pm b/lib/RT/Interface/Email/Auth/MailFrom.pm
index e733bda..cfa67d4 100644
--- a/lib/RT/Interface/Email/Auth/MailFrom.pm
+++ b/lib/RT/Interface/Email/Auth/MailFrom.pm
@@ -98,83 +98,69 @@ sub GetCurrentUser {
 
     # but before we do that, we need to make sure that the created user would have the right
     # to do what we're doing.
+    my ($right, $error) = WhichRightShouldBeChecked( %args );
+    return ( $args{'CurrentUser'}, 0 ) unless $right;
+
+    unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
+                                                Right => $right )
+             || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
+                                                 Right => $right )
+    ) {
+        $RT::Logger->debug( $error );
+        return ( $args{'CurrentUser'}, 0 );
+    }
+
+    $CurrentUser = CreateUser( undef, $Address, $Name, $Address, $args{'Message'} );
+
+    return ( $CurrentUser, 1 );
+}
+
+sub WhichRightShouldBeChecked {
+    my %args = @_;
+
+    my $qname = $args{'Queue'}->Name;
+
     if ( $args{'Ticket'} && $args{'Ticket'}->Id ) {
-        my $qname = $args{'Queue'}->Name;
         # We have a ticket. that means we're commenting or corresponding
         if ( $args{'Action'} =~ /^comment$/i ) {
-
-            # check to see whether "Everyone" or "Unprivileged users" can comment on tickets
-            unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                        Right => 'CommentOnTicket' )
-                     || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                         Right => 'CommentOnTicket' ) )
-            {
-                $RT::Logger->debug("Unprivileged users have no right to comment on ticket in queue '$qname'");
-                return ( $args{'CurrentUser'}, 0 );
-            }
+            return (
+                'CommentOnTicket',
+                "Unprivileged users have no right to comment on ticket in queue '$qname'",
+            );
         }
         elsif ( $args{'Action'} =~ /^correspond$/i ) {
-
-            # check to see whether "Everybody" or "Unprivileged users" can correspond on tickets
-            unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                        Right  => 'ReplyToTicket' )
-                     || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                         Right  => 'ReplyToTicket' ) )
-            {
-                $RT::Logger->debug("Unprivileged users have no right to reply to ticket in queue '$qname'");
-                return ( $args{'CurrentUser'}, 0 );
-            }
+            return (
+                'ReplyToTicket',
+                "Unprivileged users have no right to reply to ticket in queue '$qname'",
+            );
         }
         elsif ( $args{'Action'} =~ /^take$/i ) {
-
-            # check to see whether "Everybody" or "Unprivileged users" can correspond on tickets
-            unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                        Right  => 'OwnTicket' )
-                     || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                         Right  => 'OwnTicket' ) )
-            {
-                $RT::Logger->debug("Unprivileged users have no right to own ticket in queue '$qname'");
-                return ( $args{'CurrentUser'}, 0 );
-            }
-
+            return (
+                'OwnTicket',
+                "Unprivileged users have no right to own ticket in queue '$qname'",
+            );
         }
         elsif ( $args{'Action'} =~ /^resolve$/i ) {
-
-            # check to see whether "Everybody" or "Unprivileged users" can correspond on tickets
-            unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                        Right  => 'ModifyTicket' )
-                     || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                         Right  => 'ModifyTicket' ) )
-            {
-                $RT::Logger->debug("Unprivileged users have no right to resolve ticket in queue '$qname'");
-                return ( $args{'CurrentUser'}, 0 );
-            }
-
+            return (
+                'ModifyTicket',
+                "Unprivileged users have no right to resolve ticket in queue '$qname'",
+            );
         }
         else {
             $RT::Logger->warning("Action '". ($args{'Action'}||'') ."' is unknown");
-            return ( $args{'CurrentUser'}, 0 );
+            return;
         }
     }
 
     # We're creating a ticket
     elsif ( $args{'Queue'} && $args{'Queue'}->Id ) {
-        my $qname = $args{'Queue'}->Name;
-
         # check to see whether "Everybody" or "Unprivileged users" can create tickets in this queue
-        unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                    Right  => 'CreateTicket' )
-                 || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                     Right  => 'CreateTicket' ) )
-        {
-            $RT::Logger->debug("Unprivileged users have no right to create ticket in queue '$qname'");
-            return ( $args{'CurrentUser'}, 0 );
-        }
+        return (
+            'CreateTicket',
+            "Unprivileged users have no right to create ticket in queue '$qname'",
+        );
     }
-
-    $CurrentUser = CreateUser( undef, $Address, $Name, $Address, $args{'Message'} );
-
-    return ( $CurrentUser, 1 );
+    return;
 }
 
 RT::Base->_ImportOverlays();

commit 22972d422bc4c1735ec7db9b8fc7554d40bf3f5c
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 2 15:45:59 2012 +0400

    change how address parsers work
    
    They used to return (address, phrase) pair of first address
    in not empty header with addresses.
    
    Now in scalar context we return array reference with Email::Address
    objects.

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 909a9f4..66d569f 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1072,14 +1072,9 @@ sub ParseSenderAddressFromHead {
     my $head = shift;
 
     #Figure out who's sending this message.
-    foreach my $header ('Reply-To', 'From', 'Sender') {
-        my $addr_line = $head->get($header) || next;
-        my ($addr, $name) = ParseAddressFromHeader( $addr_line );
-        # only return if the address is not empty
-        return ($addr, $name) if $addr;
-    }
-
-    return (undef, undef);
+    return ParseFirstNotEmptyAddressHeaderFromHead(
+        $head, 'Reply-To', 'From', 'Sender',
+    );
 }
 
 =head2 ParseErrorsToAddressFromHead HEAD
@@ -1093,21 +1088,25 @@ From:, Sender)
 sub ParseErrorsToAddressFromHead {
     my $head = shift;
 
-    #Figure out who's sending this message.
-
-    foreach my $header ( 'Errors-To', 'Reply-To', 'From', 'Sender' ) {
+    # Figure out who's sending this message.
+    my $list = ParseFirstNotEmptyAddressHeaderFromHead(
+        $head, 'Errors-To', 'Reply-To', 'From', 'Sender',
+    ) or return undef;
+    return $list->[0]->address;
+}
 
-        # If there's a header of that name
-        my $headerobj = $head->get($header);
-        if ($headerobj) {
-            my ( $addr, $name ) = ParseAddressFromHeader($headerobj);
+sub ParseFirstNotEmptyAddressHeaderFromHead {
+    my ($head, @fields) = @_;
 
-            # If it's got actual useful content...
-            return ($addr) if ($addr);
-        }
+    foreach my $header ( @fields ) {
+        my $addr_line = $head->get($header) || next;
+        my $list = ParseAddressFromHeader( $addr_line ) || next;
+        return $list unless wantarray;
+        return ($list->[0]->address, $list->[0]->phrase);
     }
-}
 
+    return undef;
+}
 
 
 =head2 ParseAddressFromHeader ADDRESS
@@ -1121,14 +1120,10 @@ sub ParseAddressFromHeader {
 
     # Some broken mailers send:  ""Vincent, Jesse"" <jesse at fsck.com>. Hate
     $Addr =~ s/\"\"(.*?)\"\"/\"$1\"/g;
-    my @Addresses = RT::EmailParser->ParseEmailAddress($Addr);
-
-    my ($AddrObj) = grep ref $_, @Addresses;
-    unless ( $AddrObj ) {
-        return ( undef, undef );
-    }
-
-    return ( $AddrObj->address, $AddrObj->phrase );
+    my @addresses = grep ref $_, RT::EmailParser->ParseEmailAddress($Addr);
+    return undef unless @addresses;
+    return \@addresses unless wantarray;
+    return ($addresses[0]->address, $addresses[0]->phrase);
 }
 
 =head2 DeleteRecipientsFromHead HEAD RECIPIENTS

commit 8fb18dfe3180ccaf323adf4bee730b744b6668d2
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 2 22:55:55 2012 +0400

    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/Auth/MailFrom.pm b/lib/RT/Interface/Email/Auth/MailFrom.pm
index cfa67d4..e129e1f 100644
--- a/lib/RT/Interface/Email/Auth/MailFrom.pm
+++ b/lib/RT/Interface/Email/Auth/MailFrom.pm
@@ -66,20 +66,12 @@ sub GetCurrentUser {
 
 
     # We don't need to do any external lookups
-    my ( $Address, $Name ) = ParseSenderAddressFromHead( $args{'Message'}->head );
-    unless ( $Address ) {
+    my $addresses = ParseSenderAddressFromHead( $args{'Message'}->head );
+    unless ( $addresses ) {
         $RT::Logger->error("Couldn't find sender's address");
         return ( $args{'CurrentUser'}, -1 );
     }
 
-    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, 1 );
-    }
-
     # If the user can't be loaded, we may need to create one. Figure out the acl situation.
     my $unpriv = RT->UnprivilegedUsers();
     unless ( $unpriv->Id ) {
@@ -94,25 +86,51 @@ sub GetCurrentUser {
         return ( $args{'CurrentUser'}, -1 );
     }
 
-    $RT::Logger->debug("Going to create user with address '$Address'" );
-
-    # but before we do that, we need to make sure that the created user would have the right
-    # to do what we're doing.
-    my ($right, $error) = WhichRightShouldBeChecked( %args );
+    my ($right, $right_error) = WhichRightShouldBeChecked( %args );
     return ( $args{'CurrentUser'}, 0 ) unless $right;
 
-    unless ( $everyone->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                Right => $right )
-             || $unpriv->PrincipalObj->HasRight( Object => $args{'Queue'},
-                                                 Right => $right )
-    ) {
-        $RT::Logger->debug( $error );
-        return ( $args{'CurrentUser'}, 0 );
+    my $check_right = sub {
+        if ( my $user = shift ) {
+            return $user->PrincipalObj->HasRight(
+                Object => $args{'Queue'}, Right => $right
+            );
+        }
+        return
+            $everyone->PrincipalObj->HasRight(
+                Object => $args{'Queue'}, Right => $right
+            ) || $unpriv->PrincipalObj->HasRight(
+                Object => $args{'Queue'}, Right => $right,
+            )
+        ;
+    };
+
+    my $user;
+    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;
+        if ( $CurrentUser->Id ) {
+            $RT::Logger->debug("$addr belongs to user #". $CurrentUser->Id );
+            return ( $CurrentUser, 1 ) if $check_right->( $CurrentUser );
+
+            $user ||= $CurrentUser;
+            $RT::Logger->debug( "User has $right_error. Skipping $addr" );
+        }
+        elsif ( $check_right->() ) {
+            $RT::Logger->debug("Going to create user $addr" );
+            $CurrentUser = CreateUser(
+                undef, $addr->address, $addr->phrase, $addr->address,
+                $args{'Message'}
+            );
+            return ( $CurrentUser, 1 );
+        } else {
+            $RT::Logger->debug( "Unprivileged users have $right_error. Skipping $addr" );
+        }
     }
-
-    $CurrentUser = CreateUser( undef, $Address, $Name, $Address, $args{'Message'} );
-
-    return ( $CurrentUser, 1 );
+    $RT::Logger->debug("Sender(s) has no enough rights");
+    return $user ? ($user, 1) : ($args{'CurrentUser'}, 0);
 }
 
 sub WhichRightShouldBeChecked {
@@ -125,25 +143,25 @@ sub WhichRightShouldBeChecked {
         if ( $args{'Action'} =~ /^comment$/i ) {
             return (
                 'CommentOnTicket',
-                "Unprivileged users have no right to comment on ticket in queue '$qname'",
+                "no right to comment on ticket in queue '$qname'",
             );
         }
         elsif ( $args{'Action'} =~ /^correspond$/i ) {
             return (
                 'ReplyToTicket',
-                "Unprivileged users have no right to reply to ticket in queue '$qname'",
+                "no right to reply to ticket in queue '$qname'",
             );
         }
         elsif ( $args{'Action'} =~ /^take$/i ) {
             return (
                 'OwnTicket',
-                "Unprivileged users have no right to own ticket in queue '$qname'",
+                "no right to own ticket in queue '$qname'",
             );
         }
         elsif ( $args{'Action'} =~ /^resolve$/i ) {
             return (
                 'ModifyTicket',
-                "Unprivileged users have no right to resolve ticket in queue '$qname'",
+                "no right to resolve ticket in queue '$qname'",
             );
         }
         else {
@@ -157,7 +175,7 @@ sub WhichRightShouldBeChecked {
         # check to see whether "Everybody" or "Unprivileged users" can create tickets in this queue
         return (
             'CreateTicket',
-            "Unprivileged users have no right to create ticket in queue '$qname'",
+            "no right to create ticket in queue '$qname'",
         );
     }
     return;

commit a645597f7541cbabcf47e4580a2121eb86d7539d
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 2 22:58:33 2012 +0400

    use sender in ParseCcAddressesFromHead
    
    in case there are more than one address in reply-to

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 66d569f..4d63b1d 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1052,11 +1052,14 @@ sub ParseCcAddressesFromHead {
     my $current_address = lc $args{'CurrentUser'}->EmailAddress;
     my $user = $args{'CurrentUser'}->UserObj;
 
+    my $sender = ParseSenderAddressFromHead( $args{'Head'} );
+
     return
         grep $_ ne $current_address && !RT::EmailParser->IsRTAddress( $_ ),
         map lc $user->CanonicalizeEmailAddress( $_->address ),
-        map Email::Address->parse( $args{'Head'}->get( $_ ) ),
-        qw(To Cc);
+        ( map Email::Address->parse( $args{'Head'}->get( $_ ) ), qw(To Cc) ),
+        @{ $sender || [] }
+    ;
 }
 
 

commit d00b0982009e832f1e9701f991dd9153eae8fa63
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..d1314b6
--- /dev/null
+++ b/t/mail/multiple-reply-to.t
@@ -0,0 +1,53 @@
+use strict;
+use warnings;
+
+use RT::Test tests => 7;
+
+RT->Config->Set( ParseNewMessageForTicketCcs => 1 );
+
+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, my $tid) = RT::Test->send_via_mailgate($text);
+    is ($status >> 8, 0, "The mail gateway exited normally");
+    is ($tid, $id, "ticket updated");
+}
+
+
+1;
\ No newline at end of file

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


More information about the Rt-commit mailing list