[Rt-commit] rt branch 5.0/stop-ticket-create-modify-on-invalid-recipients created. rt-5.0.2-115-g0b6df32d41
BPS Git Server
git at git.bestpractical.com
Tue Apr 12 18:54:24 UTC 2022
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".
The branch, 5.0/stop-ticket-create-modify-on-invalid-recipients has been created
at 0b6df32d41748e998950412ba94c3e0047fdee33 (commit)
- Log -----------------------------------------------------------------
commit 0b6df32d41748e998950412ba94c3e0047fdee33
Author: Brian Conry <bconry at bestpractical.com>
Date: Thu Mar 24 12:58:44 2022 -0500
Block ticket create/update on invalid recipients
The prior behavior was to proceed with the ticket create or update,
excluding the recipients that did not look like email addresses and
could not be resolved to a principal, and without letting the user know
that some intended recipients had been omitted.
The new behavior is to block the action and give the user a message
explaining that no user could be found for what they entered.
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index 5ae39fd4e5..b5a7b4151e 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -506,6 +506,8 @@ if ($ARGS{IncludeArticleId}) {
# check email addresses for RT's
{
+ my $recipient_check = RT::Ticket->new($session{'CurrentUser'});
+
foreach my $field ( qw(Requestors Cc AdminCc) ) {
my $value = $ARGS{ $field };
next unless defined $value && length $value;
@@ -524,7 +526,14 @@ if ($ARGS{IncludeArticleId}) {
}
}
else {
- push @emails, $entry->{value};
+ my($principals, $messages) = $recipient_check->ParseInputPrincipals($entry->{value});
+ if (!$principals->[0]) {
+ push @results, loc("Couldn't add '[_1]' as [_2]: [_3]", $entry->{value}, loc($field =~ /^(.*?)s?$/), $messages->[0]);
+ $checks_failure ||= 1;
+ }
+ else {
+ push @emails, $entry->{value};
+ }
}
}
$ARGS{ $field } = join ', ', grep defined, @emails;
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index c1b7f38cf0..67c0f4e119 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -423,6 +423,8 @@ if ( $ARGS{'SubmitTicket'} ) {
# check email addresses for RT's
{
+ my $recipient_check = RT::Ticket->new($session{'CurrentUser'});
+
foreach my $field ( qw(UpdateCc UpdateBcc) ) {
my $value = $ARGS{ $field };
next unless defined $value && length $value;
@@ -433,7 +435,7 @@ if ( $ARGS{'SubmitTicket'} ) {
if ( $entry->{type} eq 'mailbox' ) {
my $email = $entry->{value};
if ( RT::EmailParser->IsRTAddress($email->address) ) {
- push @results, loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop", $email->format, loc(substr($field, 6)) );
+ push @results, loc("[_1] is an address RT receives mail at. Adding it as a 'One-time [_2]' would create a mail loop", $email->format, loc($field =~ /^Update(.*?)$/) );
$checks_failure ||= 1;
}
else {
@@ -441,7 +443,14 @@ if ( $ARGS{'SubmitTicket'} ) {
}
}
else {
- push @emails, $entry->{value};
+ my($principals, $messages) = $recipient_check->ParseInputPrincipals($entry->{value});
+ if (!$principals->[0]) {
+ push @results, loc("Couldn't add '[_1]' to 'One-time [_2]': [_3]", $entry->{value}, loc($field =~ /^Update(.*?)$/), $messages->[0]);
+ $checks_failure ||= 1;
+ }
+ else {
+ push @emails, $entry->{value};
+ }
}
}
$ARGS{ $field } = join ', ', grep defined, @emails;
diff --git a/t/web/ticket_role_input.t b/t/web/ticket_role_input.t
index e4fa7fc448..e6ebc2af39 100644
--- a/t/web/ticket_role_input.t
+++ b/t/web/ticket_role_input.t
@@ -3,6 +3,14 @@ use warnings;
use RT::Test tests => undef;
+# having this set overrides checking against individual configured addresses,
+# and the Test default value can't match something that also looks like an email address
+RT->Config->Set('RTAddressRegexp', undef);
+is( RT->Config->Get('RTAddressRegexp'), undef, 'global RTAddressRegexp is not set');
+
+RT->Config->Set('CommentAddress', 'rt-comment at example.com');
+is( RT->Config->Get('CommentAddress'), 'rt-comment at example.com', 'global comment address set');
+
my ( $baseurl, $m ) = RT::Test->started_ok;
ok $m->login, 'logged in as root';
my $root = RT::User->new( RT->SystemUser );
@@ -26,6 +34,8 @@ ok( $group_admin_user->id, 'created group admin user' );
my $queue = RT::Test->load_or_create_queue( Name => 'General' );
ok $queue->id, 'loaded queue General';
+# test the success cases
+
diag "Test ticket create page";
{
$m->goto_create_ticket( $queue );
@@ -170,4 +180,73 @@ diag "Test ticket bulk update page";
}
}
+# make sure that any warnings from the preceeding (which shouldn't happen) don't affect the tests that follow
+$m->no_warnings_ok;
+
+# test the failure cases
+ok( $queue->SetCorrespondAddress('rt-general at example.com'), 'Set queue correspond address' );
+
+diag "Test ticket create page (failures)";
+{
+ $m->goto_create_ticket( $queue );
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketCreate',
+ fields => {
+ Subject => 'test input errors on create',
+ Content => 'test content',
+ Requestors => 'sybil, group:think, rt-general at example.com, rt-comment at example.com',
+ Cc => 'sybil, group:think, rt-general at example.com, rt-comment at example.com',
+ AdminCc => 'sybil, group:think, rt-general at example.com, rt-comment at example.com',
+ },
+ button => 'SubmitTicket',
+ },
+ 'submit form TicketCreate'
+ );
+
+ $m->next_warning_like( qr/^Couldn't load (?:user from value sybil|group from value group:think), Couldn't find row$/, 'found expected warning' ) for 1 .. 6;
+
+ foreach my $role (qw(Requestor Cc AdminCc)) {
+ $m->text_like( qr/Couldn't add 'sybil' as $role/, "expected user warning: sybil $role" );
+ $m->text_like( qr/Couldn't add 'group:think' as $role/, "expected user warning: group:think $role" );
+
+ $m->text_like( qr/rt-general\@example.com is an address RT receives mail at. Adding it as a '$role' would create a mail loop/, "expected user warning: rt-general\@example.com $role" );
+ $m->text_like( qr/rt-comment\@example.com is an address RT receives mail at. Adding it as a '$role' would create a mail loop/, "expected user warning: rt-comment\@example.com $role" );
+ }
+}
+
+diag "Test ticket update page (failures)";
+{
+ my $ticket = RT::Test->create_ticket(
+ Queue => $queue,
+ Subject => 'test inputs on update',
+ Content => 'test content',
+ );
+ $m->goto_ticket( $ticket->id, 'Update' );
+
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketUpdate',
+ fields => {
+ UpdateContent => 'test content',
+ UpdateCc => 'sybil, group:think, rt-general at example.com, rt-comment at example.com',
+ UpdateBcc => 'sybil, group:think, rt-general at example.com, rt-comment at example.com',
+ },
+ button => 'SubmitTicket',
+ },
+ 'submit form TicketCreate'
+ );
+
+ $m->next_warning_like( qr/^Couldn't load (?:user from value sybil|group from value group:think), Couldn't find row$/, 'found expected warning' ) for 1 .. 4;
+
+ foreach my $role (qw(Cc Bcc)) {
+ $m->text_like( qr/Couldn't add 'sybil' to 'One-time $role'/, "expected user warning: sybil $role" );
+ $m->text_like( qr/Couldn't add 'group:think' to 'One-time $role'/, "expected user warning: group:think $role" );
+
+ $m->text_like( qr/rt-general\@example.com is an address RT receives mail at. Adding it as a 'One-time $role' would create a mail loop/, "expected user warning: rt-general\@example.com $role" );
+ $m->text_like( qr/rt-comment\@example.com is an address RT receives mail at. Adding it as a 'One-time $role' would create a mail loop/, "expected user warning: rt-comment\@example.com $role" );
+ }
+}
+
+
done_testing;
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list