[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