[Rt-commit] rt branch 5.0/delete-rt-address-from-roles created. rt-5.0.2-38-gde51092fb7

[BPS Git Server]

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/delete-rt-address-from-roles has been created
        at  de51092fb721d61e5eb22840947d3003fabc4019 (commit)

- Log -----------------------------------------------------------------
commit de51092fb721d61e5eb22840947d3003fabc4019
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Nov 18 07:55:31 2021 +0800

    Test the deletion of RT addresses from ticket roles

diff --git a/t/api/ticket.t b/t/api/ticket.t
index 4ce1f9a484..6c101032e1 100644
--- a/t/api/ticket.t
+++ b/t/api/ticket.t
@@ -363,4 +363,25 @@ diag("Test LazyRoleGroups");
     }
 }
 
+diag "Delete role members with RT internal addresses";
+{
+    RT->Config->Set( 'RTAddressRegexp', undef );
+    my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+    ok( $queue->SetCorrespondAddress('test at localhost') );
+    is( $queue->CorrespondAddress, 'test at localhost', 'Set queue reply address' );
+    my $t = RT::Ticket->new( RT->SystemUser );
+    my ($ok) = $t->Create(
+        Queue     => 'General',
+        Subject   => 'Delete RT address test from role',
+        Requestor => 'test at localhost',
+    );
+    ok( $ok, "Tickets can be created with an queue reply address as requestor" );
+    is( $t->RequestorAddresses, 'test at localhost', 'Queue reply address is in requestor' );
+
+    ( $ok, my $msg ) = $t->DeleteRoleMember( Type => 'Requestor', User => 'test at localhost' );
+    ok( $ok, $msg );
+    is( $t->RequestorAddresses, '', 'Queue reply address is not in requestor any more' );
+    ok( $queue->CorrespondAddress('') );
+}
+
 done_testing;

commit 9e18d382a0b10f67ace466c07a38bb4b35670f27
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Nov 16 03:43:24 2021 +0800

    Allow to delete RT addresses from roles
    
    RT users with internal addresses(e.g. the queue reply address) can
    create tickets(they will be added as requestors) as long as they have
    "CreateTicket" right. This is an unusual setup but does work.
    
    Previously people were able to delete these addresses from requestors,
    which isn't true any more in RT 4.4.5 and RT 5.0.2, as they include
    df2dd2e3bf to reuse CanonicalizePrincipal in DeleteRoleMember, which
    denies internal addresses.
    
    This commit tweaks CanonicalizePrincipal and DeleteRoleMember to
    continue accepting internal addresses.

diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index 4b7449d22f..9a06e0e1b8 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -405,24 +405,14 @@ The Name of an L<RT::Group>.
 
 sub CanonicalizePrincipal {
     my $self = shift;
-    my %args = (@_);
+    my %args = (ExcludeRTAddress => 1, @_);
 
     return (0, $self->loc("One, and only one, of Principal/PrincipalId/User/Group is required"))
         if 1 != grep { $_ } @args{qw/Principal PrincipalId User Group/};
 
-    if ($args{Principal}) {
-        return $args{Principal};
-    }
-    elsif ($args{PrincipalId}) {
-        # Check the PrincipalId for loops
-        my $principal = RT::Principal->new( $self->CurrentUser );
-        $principal->Load($args{'PrincipalId'});
-        if ( $principal->id and $principal->IsUser and my $email = $principal->Object->EmailAddress ) {
-            return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
-                                  $email, $self->loc($args{Type})))
-                if RT::EmailParser->IsRTAddress( $email );
-        }
-    } else {
+    $args{PrincipalId} = $args{Principal}->Id if $args{Principal};
+
+    if ( !$args{PrincipalId} ) {
         if ( ( $args{User} || '' ) =~ /^\s*group\s*:\s*(\S.*?)\s*$/i ) {
             $args{Group} = $1;
             delete $args{User};
@@ -433,7 +423,7 @@ sub CanonicalizePrincipal {
             # Sanity check the address
             return (0, $self->loc("[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
                                   $name, $self->loc($args{Type}) ))
-                if RT::EmailParser->IsRTAddress( $name );
+                if $args{ExcludeRTAddress} && RT::EmailParser->IsRTAddress( $name );
 
             # Create as the SystemUser, not the current user
             my $user = RT::User->new(RT->SystemUser);
@@ -465,6 +455,20 @@ sub CanonicalizePrincipal {
     my $principal = RT::Principal->new( $self->CurrentUser );
     $principal->Load( $args{PrincipalId} );
 
+    if (    $args{ExcludeRTAddress}
+        and $principal->Id
+        and $principal->IsUser
+        and my $email = $principal->Object->EmailAddress )
+    {
+        return (
+            0,
+            $self->loc(
+                "[_1] is an address RT receives mail at. Adding it as a '[_2]' would create a mail loop",
+                $email, $self->loc( $args{Type} )
+            )
+        ) if RT::EmailParser->IsRTAddress($email);
+    }
+
     return $principal;
 }
 
@@ -570,7 +574,7 @@ sub DeleteRoleMember {
     return (0, $self->loc("That role is invalid for this object"))
         unless $args{Type} and $self->HasRole($args{Type});
 
-    my ($principal, $msg) = $self->CanonicalizePrincipal(%args);
+    my ($principal, $msg) = $self->CanonicalizePrincipal(%args, ExcludeRTAddress => 0);
     return (0, $msg) if !$principal;
 
     my $acl = delete $args{ACL};

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


hooks/post-receive
-- 
rt