TimeWorked-like custom fields, RT#11168
[freeside.git] / rt / lib / RT / ACE_Overlay.pm
index 00a7157..824429f 100644 (file)
@@ -1,38 +1,40 @@
-# {{{ BEGIN BPS TAGGED BLOCK
-# 
+# BEGIN BPS TAGGED BLOCK {{{
+#
 # COPYRIGHT:
-#  
-# This software is Copyright (c) 1996-2004 Best Practical Solutions, LLC 
-#                                          <jesse@bestpractical.com>
-# 
+#
+# This software is Copyright (c) 1996-2011 Best Practical Solutions, LLC
+#                                          <sales@bestpractical.com>
+#
 # (Except where explicitly superseded by other copyright notices)
-# 
-# 
+#
+#
 # LICENSE:
-# 
+#
 # This work is made available to you under the terms of Version 2 of
 # the GNU General Public License. A copy of that license should have
 # been provided with this software, but in any event can be snarfed
 # from www.gnu.org.
-# 
+#
 # This work is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # General Public License for more details.
-# 
+#
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
-# 
-# 
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
 # CONTRIBUTION SUBMISSION POLICY:
-# 
+#
 # (The following paragraph is not intended to limit the rights granted
 # to you to modify and distribute this software under the terms of
 # the GNU General Public License and is only of importance to you if
 # you choose to contribute your changes and enhancements to the
 # community by submitting them to Best Practical Solutions, LLC.)
-# 
+#
 # By intentionally submitting any modifications, corrections or
 # derivatives to this work, or any other work intended for use with
 # Request Tracker, to Best Practical Solutions, LLC, you confirm that
@@ -41,8 +43,9 @@
 # royalty-free, perpetual, license to use, copy, create derivative
 # works based on those contributions, and sublicense and distribute
 # those contributions and any derivatives thereof.
-# 
-# }}} END BPS TAGGED BLOCK
+#
+# END BPS TAGGED BLOCK }}}
+
 =head1 SYNOPSIS
 
   use RT::ACE;
 
 =head1 METHODS
 
-=begin testing
 
-ok(require RT::ACE);
+=cut
 
-=end testing
 
-=cut
+package RT::ACE;
 
 use strict;
 no warnings qw(redefine);
@@ -84,15 +85,6 @@ use vars qw (
 # to real people or groups
 
 
-=begin testing
-
-my $Queue = RT::Queue->new($RT::SystemUser);
-
-is ($Queue->AvailableRights->{'DeleteTicket'} , 'Delete tickets', "Found the delete ticket right");
-is ($RT::System->AvailableRights->{'SuperUser'},  'Do anything and everything', "Found the superuser right");
-
-
-=end testing
 
 =cut
 
@@ -129,8 +121,8 @@ Load an ACE by specifying a paramhash with the following fields:
 
             OR
 
-        ObjectType => undef,
-        ObjectId => undef
+             ObjectType => undef,
+             ObjectId => undef
 
 =cut
 
@@ -144,6 +136,14 @@ sub LoadByValues {
                  ObjectType    => undef,
                  @_ );
 
+    if ( $args{'RightName'} ) {
+        my $canonic_name = $self->CanonicalizeRightName( $args{'RightName'} );
+        unless ( $canonic_name ) {
+            return ( 0, $self->loc("Invalid right. Couldn't canonicalize right '[_1]'", $args{'RightName'}) );
+        }
+        $args{'RightName'} = $canonic_name;
+    }
+
     my $princ_obj;
     ( $princ_obj, $args{'PrincipalType'} ) =
       $self->_CanonicalizePrincipal( $args{'PrincipalId'},
@@ -155,16 +155,9 @@ sub LoadByValues {
         );
     }
 
-    my ($object_type, $object_id);
-    
-    if ($args{'Object'} && UNIVERSAL::can($args{'Object'},'id')) {
-        $object_type = ref($args{'Object'});
-        $object_id = $args{'Object'}->id;
-    } elsif ($args{'ObjectId'} || $args{'ObjectType'}) {
-        $object_type = $args{'ObjectType'};
-        $object_id = $args{'ObjectId'};
-    } else {
-            return ( 0, $self->loc("System error. Right not granted.") );
+    my ($object, $object_type, $object_id) = $self->_ParseObjectArg( %args );
+    unless( $object ) {
+       return ( 0, $self->loc("System error. Right not granted.") );
     }
 
     $self->LoadByCols( PrincipalId   => $princ_obj->Id,
@@ -208,15 +201,36 @@ PARAMS is a parameter hash with the following elements:
    ObjectType => the type of the object in question (ref ($object))
    ObjectId => the id of the object in question $object->Id
 
+
+
+   Returns a tuple of (STATUS, MESSAGE);  If the call succeeded, STATUS is true. Otherwise it's false.
+
+
+
 =cut
 
 sub Create {
     my $self = shift;
-    my %args = ( PrincipalId   => undef,
-                 PrincipalType => undef,
-                 RightName     => undef,
-                 Object    => $RT::System,
-                 @_ );
+    my %args = (
+        PrincipalId   => undef,
+        PrincipalType => undef,
+        RightName     => undef,
+        Object        => undef,
+        @_
+    );
+
+    unless ( $args{'RightName'} ) {
+        return ( 0, $self->loc('No right specified') );
+    }
+
+    #if we haven't specified any sort of right, we're talking about a global right
+    if (!defined $args{'Object'} && !defined $args{'ObjectId'} && !defined $args{'ObjectType'}) {
+        $args{'Object'} = $RT::System;
+    }
+    ($args{'Object'}, $args{'ObjectType'}, $args{'ObjectId'}) = $self->_ParseObjectArg( %args );
+    unless( $args{'Object'} ) {
+       return ( 0, $self->loc("System error. Right not granted.") );
+    }
 
     # {{{ Validate the principal
     my $princ_obj;
@@ -232,17 +246,6 @@ sub Create {
 
     # }}}
 
-
-    if ($args{'Object'} && ($args{'ObjectId'} || $args{'ObjectType'})) {
-        use Carp;
-        $RT::Logger->crit(Carp::cluck("ACE::Create called with an ObjectType or an ObjectId"));
-    }
-
-
-    
-    unless ($args{'Object'} && UNIVERSAL::can($args{'Object'},'id')) {
-            return ( 0, $self->loc("System error. Right not granted.") );
-    }
     # {{{ Check the ACL
 
     if (ref( $args{'Object'}) eq 'RT::Group' ) {
@@ -261,48 +264,31 @@ sub Create {
     # }}}
 
     # {{{ Canonicalize and check the right name
-    unless ( $args{'RightName'} ) {
-        return ( 0, $self->loc('Invalid right') );
+    my $canonic_name = $self->CanonicalizeRightName( $args{'RightName'} );
+    unless ( $canonic_name ) {
+        return ( 0, $self->loc("Invalid right. Couldn't canonicalize right '[_1]'", $args{'RightName'}) );
     }
-
-    $args{'RightName'} = $self->CanonicalizeRightName( $args{'RightName'} );
+    $args{'RightName'} = $canonic_name;
 
     #check if it's a valid RightName
-    if ( ref ($args{'Object'} eq 'RT::Queue'  )) {
-        unless ( exists $args{'Object'}->AvailableRights->{ $args{'RightName'} } ) {
-            $RT::Logger->warning("Couldn't validate right name". $args{'RightName'});
-            return ( 0, $self->loc('Invalid right') );
-        }
-    }
-    elsif ( ref ($args{'Object'} eq 'RT::Group'  )) {
-        unless ( exists $args{'Object'}->AvailableRights->{ $args{'RightName'} } ) {
-            $RT::Logger->warning("Couldn't validate group right name". $args{'RightName'});
+    if ( $args{'Object'}->can('AvailableRights') ) {
+        my $available = $args{'Object'}->AvailableRights;
+        unless ( grep $_ eq $args{'RightName'}, map $self->CanonicalizeRightName( $_ ), keys %$available ) {
+            $RT::Logger->warning(
+                "Couldn't validate right name '$args{'RightName'}'"
+                ." for object of ". ref( $args{'Object'} ) ." class"
+            );
             return ( 0, $self->loc('Invalid right') );
         }
     }
-    elsif ( ref ($args{'Object'} eq 'RT::System'  )) {
-        my $q = RT::Queue->new($self->CurrentUser);
-        my $g = RT::Group->new($self->CurrentUser);
-
-        unless (( exists $g->AvailableRights->{ $args{'RightName'} } )
-        || ( exists $g->AvailableRights->{ $args{'RightName'} } )
-        || ( exists $RT::System->AvailableRights->{ $args{'RightName'} } ) ) {
-            $RT::Logger->warning("Couldn't validate system right name - ". $args{'RightName'});
-            return ( 0, $self->loc('Invalid right') );
-        }
-    }
-
-    unless ( $args{'RightName'} ) {
-        return ( 0, $self->loc('Invalid right') );
-    }
     # }}}
 
     # Make sure the right doesn't already exist.
     $self->LoadByCols( PrincipalId   => $princ_obj->id,
                        PrincipalType => $args{'PrincipalType'},
                        RightName     => $args{'RightName'},
-                       ObjectType    => ref($args{'Object'}),
-                       ObjectId      => $args{'Object'}->id,
+                       ObjectType    => $args{'ObjectType'},
+                       ObjectId      => $args{'ObjectId'},
                        DelegatedBy   => 0,
                        DelegatedFrom => 0 );
     if ( $self->Id ) {
@@ -318,9 +304,9 @@ sub Create {
                                    DelegatedFrom => 0 );
 
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
-    RT::Principal->_InvalidateACLCache();
+    RT::Principal->InvalidateACLCache();
 
-    if ( $id > 0 ) {
+    if ( $id ) {
         return ( $id, $self->loc('Right Granted') );
     }
     else {
@@ -342,216 +328,6 @@ or doesn't have the right to delegate rights.
 
 Always returns a tuple of (ReturnValue, Message)
 
-=begin testing
-
-use_ok(RT::User);
-my $user_a = RT::User->new($RT::SystemUser);
-$user_a->Create( Name => 'DelegationA', Privileged => 1);
-ok ($user_a->Id, "Created delegation user a");
-
-my $user_b = RT::User->new($RT::SystemUser);
-$user_b->Create( Name => 'DelegationB', Privileged => 1);
-ok ($user_b->Id, "Created delegation user b");
-
-
-use_ok(RT::Queue);
-my $q = RT::Queue->new($RT::SystemUser);
-$q->Create(Name =>'DelegationTest');
-ok ($q->Id, "Created a delegation test queue");
-
-
-#------ First, we test whether a user can delegate a right that's been granted to him personally 
-my ($val, $msg) = $user_a->PrincipalObj->GrantRight(Object => $RT::System, Right => 'AdminOwnPersonalGroups');
-ok($val, $msg);
-
-($val, $msg) = $user_a->PrincipalObj->GrantRight(Object =>$q, Right => 'OwnTicket');
-ok($val, $msg);
-
-ok($user_a->HasRight( Object => $RT::System, Right => 'AdminOwnPersonalGroups')    ,"user a has the right 'AdminOwnPersonalGroups' directly");
-
-my $a_delegates = RT::Group->new($user_a);
-$a_delegates->CreatePersonalGroup(Name => 'Delegates');
-ok( $a_delegates->Id   ,"user a creates a personal group 'Delegates'");
-ok( $a_delegates->AddMember($user_b->PrincipalId)   ,"user a adds user b to personal group 'delegates'");
-
-ok( !$user_b->HasRight(Right => 'OwnTicket', Object => $q)    ,"user b does not have the right to OwnTicket' in queue 'DelegationTest'");
-ok(  $user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"user a has the right to 'OwnTicket' in queue 'DelegationTest'");
-ok(!$user_a->HasRight( Object => $RT::System, Right => 'DelegateRights')    ,"user a does not have the right 'delegate rights'");
-
-
-my $own_ticket_ace = RT::ACE->new($user_a);
-my $user_a_equiv_group = RT::Group->new($user_a);
-$user_a_equiv_group->LoadACLEquivalenceGroup($user_a->PrincipalObj);
-ok ($user_a_equiv_group->Id, "Loaded the user A acl equivalence group");
-my $user_b_equiv_group = RT::Group->new($user_b);
-$user_b_equiv_group->LoadACLEquivalenceGroup($user_b->PrincipalObj);
-ok ($user_b_equiv_group->Id, "Loaded the user B acl equivalence group");
-$own_ticket_ace->LoadByValues( PrincipalType => 'Group', PrincipalId => $user_a_equiv_group->PrincipalId, Object=>$q, RightName => 'OwnTicket');
-
-ok ($own_ticket_ace->Id, "Found the ACE we want to test with for now");
-
-
-($val, $msg) = $own_ticket_ace->Delegate(PrincipalId => $a_delegates->PrincipalId)  ;
-ok( !$val ,"user a tries and fails to delegate the right 'ownticket' in queue 'DelegationTest' to personal group 'delegates' - $msg");
-
-
-($val, $msg) = $user_a->PrincipalObj->GrantRight( Right => 'DelegateRights');
-ok($val, "user a is granted the right to 'delegate rights' - $msg");
-
-ok($user_a->HasRight( Object => $RT::System, Right => 'DelegateRights')    ,"user a has the right 'AdminOwnPersonalGroups' directly");
-
-($val, $msg) = $own_ticket_ace->Delegate(PrincipalId => $a_delegates->PrincipalId) ;
-
-ok( $val    ,"user a tries and succeeds to delegate the right 'ownticket' in queue 'DelegationTest' to personal group 'delegates' - $msg");
-ok(  $user_b->HasRight(Right => 'OwnTicket', Object => $q)  ,"user b has the right to own tickets in queue 'DelegationTest'");
-my $delegated_ace = RT::ACE->new($user_a);
-$delegated_ace->LoadByValues ( Object => $q, RightName => 'OwnTicket', PrincipalType => 'Group',
-PrincipalId => $a_delegates->PrincipalId, DelegatedBy => $user_a->PrincipalId, DelegatedFrom => $own_ticket_ace->Id);
-ok ($delegated_ace->Id, "Found the delegated ACE");
-
-ok(    $a_delegates->DeleteMember($user_b->PrincipalId)  ,"user a removes b from pg 'delegates'");
-ok(  !$user_b->HasRight(Right => 'OwnTicket', Object => $q)  ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-ok(  $a_delegates->AddMember($user_b->PrincipalId)    ,"user a adds user b to personal group 'delegates'");
-ok(   $user_b->HasRight(Right => 'OwnTicket', Object=> $q) ,"user b has the right to own tickets in queue 'DelegationTest'");
-ok(   $delegated_ace->Delete ,"user a revokes pg 'delegates' right to 'OwnTickets' in queue 'DelegationTest'");
-ok( ! $user_b->HasRight(Right => 'OwnTicket', Object => $q)   ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-
-($val, $msg) = $own_ticket_ace->Delegate(PrincipalId => $a_delegates->PrincipalId)  ;
-ok(  $val  ,"user a delegates pg 'delegates' right to 'OwnTickets' in queue 'DelegationTest' - $msg");
-
-ok( $user_a->HasRight(Right => 'OwnTicket', Object => $q)    ,"user a does not have the right to own tickets in queue 'DelegationTest'");
-
-($val, $msg) = $user_a->PrincipalObj->RevokeRight(Object=>$q, Right => 'OwnTicket');
-ok($val, "Revoked user a's right to own tickets in queue 'DelegationTest". $msg);
-
-ok( !$user_a->HasRight(Right => 'OwnTicket', Object => $q)    ,"user a does not have the right to own tickets in queue 'DelegationTest'");
-
- ok( !$user_b->HasRight(Right => 'OwnTicket', Object => $q)   ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-
-($val, $msg) = $user_a->PrincipalObj->GrantRight(Object=>$q, Right => 'OwnTicket');
-ok($val, $msg);
-
- ok( $user_a->HasRight(Right => 'OwnTicket', Object => $q)   ,"user a has the right to own tickets in queue 'DelegationTest'");
-
- ok(  !$user_b->HasRight(Right => 'OwnTicket', Object => $q)  ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-
-# {{{ get back to a known clean state 
-($val, $msg) = $user_a->PrincipalObj->RevokeRight( Object => $q, Right => 'OwnTicket');
-ok($val, "Revoked user a's right to own tickets in queue 'DelegationTest -". $msg);
-ok( !$user_a->HasRight(Right => 'OwnTicket', Object => $q)    ,"make sure that user a can't own tickets in queue 'DelegationTest'");
-# }}}
-
-
-# {{{ Set up some groups and membership
-my $del1 = RT::Group->new($RT::SystemUser);
-($val, $msg) = $del1->CreateUserDefinedGroup(Name => 'Del1');
-ok( $val   ,"create a group del1 - $msg");
-
-my $del2 = RT::Group->new($RT::SystemUser);
-($val, $msg) = $del2->CreateUserDefinedGroup(Name => 'Del2');
-ok( $val   ,"create a group del2 - $msg");
-($val, $msg) = $del1->AddMember($del2->PrincipalId);
-ok( $val,"make del2 a member of del1 - $msg");
-
-my $del2a = RT::Group->new($RT::SystemUser);
-($val, $msg) = $del2a->CreateUserDefinedGroup(Name => 'Del2a');
-ok( $val   ,"create a group del2a - $msg");
-($val, $msg) = $del2->AddMember($del2a->PrincipalId);  
-ok($val    ,"make del2a a member of del2 - $msg");
-
-my $del2b = RT::Group->new($RT::SystemUser);
-($val, $msg) = $del2b->CreateUserDefinedGroup(Name => 'Del2b');
-ok( $val   ,"create a group del2b - $msg");
-($val, $msg) = $del2->AddMember($del2b->PrincipalId);  
-ok($val    ,"make del2b a member of del2 - $msg");
-
-($val, $msg) = $del2->AddMember($user_a->PrincipalId) ;
-ok($val,"make 'user a' a member of del2 - $msg");
-
-($val, $msg) = $del2b->AddMember($user_a->PrincipalId) ;
-ok($val,"make 'user a' a member of del2b - $msg");
-
-# }}}
-
-# {{{ Grant a right to a group and make sure that a submember can delegate the right and that it does not get yanked
-# when a user is removed as a submember, when they're a sumember through another path 
-($val, $msg) = $del1->PrincipalObj->GrantRight( Object=> $q, Right => 'OwnTicket');
-ok( $val   ,"grant del1  the right to 'OwnTicket' in queue 'DelegationTest' - $msg");
-
-ok(  $user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"make sure that user a can own tickets in queue 'DelegationTest'");
-
-my $group_ace= RT::ACE->new($user_a);
-$group_ace->LoadByValues( PrincipalType => 'Group', PrincipalId => $del1->PrincipalId, Object => $q, RightName => 'OwnTicket');
-
-ok ($group_ace->Id, "Found the ACE we want to test with for now");
-
-($val, $msg) = $group_ace->Delegate(PrincipalId => $a_delegates->PrincipalId);
-
-ok( $val   ,"user a tries and succeeds to delegate the right 'ownticket' in queue 'DelegationTest' to personal group 'delegates' - $msg");
-ok(  $user_b->HasRight(Right => 'OwnTicket', Object => $q)  ,"user b has the right to own tickets in queue 'DelegationTest'");
-
-
-($val, $msg) = $del2b->DeleteMember($user_a->PrincipalId);
-ok( $val   ,"remove user a from group del2b - $msg");
-ok(  $user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"user a has the right to own tickets in queue 'DelegationTest'");
-ok( $user_b->HasRight(Right => 'OwnTicket', Object => $q)    ,"user b has the right to own tickets in queue 'DelegationTest'");
-
-# }}}
-
-# {{{ When a  user is removed froom a group by the only path they're in there by, make sure the delegations go away
-($val, $msg) = $del2->DeleteMember($user_a->PrincipalId);
-ok( $val   ,"remove user a from group del2 - $msg");
-ok(  !$user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"user a does not have the right to own tickets in queue 'DelegationTest' ");
-ok(  !$user_b->HasRight(Right => 'OwnTicket', Object => $q)  ,"user b does not have the right to own tickets in queue 'DelegationTest' ");
-# }}}
-
-($val, $msg) = $del2->AddMember($user_a->PrincipalId);
-ok( $val   ,"make user a a member of group del2 - $msg");
-
-($val, $msg) = $del2->PrincipalObj->GrantRight(Object=>$q, Right => 'OwnTicket');
-ok($val, "grant the right 'own tickets' in queue 'DelegationTest' to group del2 - $msg");
-
-my $del2_right = RT::ACE->new($user_a);
-$del2_right->LoadByValues( PrincipalId => $del2->PrincipalId, PrincipalType => 'Group', Object => $q, RightName => 'OwnTicket');
-ok ($del2_right->Id, "Found the right");
-
-($val, $msg) = $del2_right->Delegate(PrincipalId => $a_delegates->PrincipalId);
-ok( $val   ,"user a tries and succeeds to delegate the right 'ownticket' in queue 'DelegationTest' gotten via del2 to personal group 'delegates' - $msg");
-
-# They have it via del1 and del2
-ok( $user_a->HasRight(Right => 'OwnTicket', Object => $q)   ,"user b has the right to own tickets in queue 'DelegationTest'");
-
-
-($val, $msg) = $del2->PrincipalObj->RevokeRight(Object=>$q, Right => 'OwnTicket');
-ok($val, "revoke the right 'own tickets' in queue 'DelegationTest' to group del2 - $msg");
-ok(  $user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"user a does has the right to own tickets in queue 'DelegationTest' via del1");
-ok(  !$user_b->HasRight(Right => 'OwnTicket', Object => $q)   ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-
-($val, $msg) = $del2->PrincipalObj->GrantRight(Object=>$q, Right => 'OwnTicket');
-ok($val, "grant the right 'own tickets' in queue 'DelegationTest' to group del2 - $msg");
-
-
-$group_ace= RT::ACE->new($user_a);
-$group_ace->LoadByValues( PrincipalType => 'Group', PrincipalId => $del1->PrincipalId, Object=>$q, RightName => 'OwnTicket');
-
-ok ($group_ace->Id, "Found the ACE we want to test with for now");
-
-($val, $msg) = $group_ace->Delegate(PrincipalId => $a_delegates->PrincipalId);
-
-ok( $val   ,"user a tries and succeeds to delegate the right 'ownticket' in queue 'DelegationTest' to personal group 'delegates' - $msg");
-
-ok( $user_b->HasRight(Right => 'OwnTicket', Object => $q)    ,"user b has the right to own tickets in queue 'DelegationTest'");
-
-($val, $msg) = $del2->DeleteMember($user_a->PrincipalId);
-ok( $val   ,"remove user a from group del2 - $msg");
-
-ok(  !$user_a->HasRight(Right => 'OwnTicket', Object => $q)  ,"user a does not have the right to own tickets in queue 'DelegationTest'");
-
-ok(  !$user_b->HasRight(Right => 'OwnTicket', Object => $q)   ,"user b does not have the right to own tickets in queue 'DelegationTest'");
-
-
-
-=end testing
 
 =cut
 
@@ -628,7 +404,7 @@ sub Delegate {
 
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
     # TODO what about the groups key cache?
-    RT::Principal->_InvalidateACLCache();
+    RT::Principal->InvalidateACLCache();
 
     if ( $id > 0 ) {
         return ( $id, $self->loc('Right Delegated') );
@@ -691,7 +467,7 @@ sub _Delete {
     while ( my $delegated_ace = $delegated_from_this->Next ) {
         ( $delete_succeeded, $submsg ) =
           $delegated_ace->_Delete( InsideTransaction => 1 );
-        last if ($delete_succeeded);
+        last unless ($delete_succeeded);
     }
 
     unless ($delete_succeeded) {
@@ -701,18 +477,23 @@ sub _Delete {
 
     my ( $val, $msg ) = $self->SUPER::Delete(@_);
 
-    #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
-    # TODO what about the groups key cache?
-    RT::Principal->_InvalidateACLCache();
+    # If we're revoking delegation rights (see above), we may need to
+    # revoke all rights delegated by the recipient.
+    if ($val and ($self->RightName() eq 'DelegateRights' or
+                 $self->RightName() eq 'SuperUser')) {
+       $val = $self->PrincipalObj->_CleanupInvalidDelegations( InsideTransaction => 1 );
+    }
 
     if ($val) {
+       #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
+       # TODO what about the groups key cache?
+       RT::Principal->InvalidateACLCache();
         $RT::Handle->Commit() unless $InsideTransaction;
         return ( $val, $self->loc('Right revoked') );
     }
-    else {
-        $RT::Handle->Rollback() unless $InsideTransaction;
-        return ( 0, $self->loc('Right could not be revoked') );
-    }
+
+    $RT::Handle->Rollback() unless $InsideTransaction;
+    return ( 0, $self->loc('Right could not be revoked') );
 }
 
 # }}}
@@ -759,6 +540,20 @@ sub _BootstrapCreate {
 
 # {{{ sub CanonicalizeRightName
 
+sub RightName {
+    my $self = shift;
+    my $val = $self->_Value('RightName');
+    return $val unless $val;
+
+    my $available = $self->Object->AvailableRights;
+    foreach my $right ( keys %$available ) {
+        return $right if $val eq $self->CanonicalizeRightName($right);
+    }
+
+    $RT::Logger->error("Invalid right. Couldn't canonicalize right '$val'");
+    return $val;
+}
+
 =head2 CanonicalizeRightName <RIGHT>
 
 Takes a queue or system right name in any case and returns it in
@@ -768,14 +563,7 @@ the correct case. If it's not found, will return undef.
 
 sub CanonicalizeRightName {
     my $self  = shift;
-    my $right = shift;
-    $right = lc $right;
-    if ( exists $LOWERCASERIGHTNAMES{"$right"} ) {
-        return ( $LOWERCASERIGHTNAMES{"$right"} );
-    }
-    else {
-        return (undef);
-    }
+    return $LOWERCASERIGHTNAMES{ lc shift };
 }
 
 # }}}
@@ -812,7 +600,7 @@ sub Object {
     else {
         $RT::Logger->warning( "$self -> Object called for an object "
                               . "of an unknown type:"
-                              . $self->ObjectType );
+                              . $self->__Value('ObjectType') );
         return (undef);
     }
 }
@@ -896,14 +684,14 @@ Returns a tuple of  (RT::Principal, PrincipalType)  for the principal we really
 sub _CanonicalizePrincipal {
     my $self       = shift;
     my $princ_id   = shift;
-    my $princ_type = shift;
+    my $princ_type = shift || '';
 
     my $princ_obj = RT::Principal->new($RT::SystemUser);
     $princ_obj->Load($princ_id);
 
     unless ( $princ_obj->Id ) {
         use Carp;
-        $RT::Logger->crit(Carp::cluck);
+        $RT::Logger->crit(Carp::longmess);
         $RT::Logger->crit("Can't load a principal for id $princ_id");
         return ( $princ_obj, undef );
     }
@@ -914,9 +702,8 @@ sub _CanonicalizePrincipal {
         my $equiv_group = RT::Group->new( $self->CurrentUser );
         $equiv_group->LoadACLEquivalenceGroup($princ_obj);
         unless ( $equiv_group->Id ) {
-            $RT::Logger->crit(
-                 "No ACL equiv group for princ " . $self->__Value('ObjectId') );
-            return ( 0, $self->loc('System error. Right not granted.') );
+            $RT::Logger->crit( "No ACL equiv group for princ " . $princ_obj->id );
+            return ( RT::Principal->new($RT::SystemUser), undef );
         }
         $princ_obj  = $equiv_group->PrincipalObj();
         $princ_type = 'Group';
@@ -925,5 +712,32 @@ sub _CanonicalizePrincipal {
     return ( $princ_obj, $princ_type );
 }
 
+sub _ParseObjectArg {
+    my $self = shift;
+    my %args = ( Object    => undef,
+                 ObjectId    => undef,
+                 ObjectType    => undef,
+                 @_ );
+
+    if( $args{'Object'} && ($args{'ObjectId'} || $args{'ObjectType'}) ) {
+       $RT::Logger->crit( "Method called with an ObjectType or an ObjectId and Object args" );
+       return ();
+    } elsif( $args{'Object'} && !UNIVERSAL::can($args{'Object'},'id') ) {
+       $RT::Logger->crit( "Method called called Object that has no id method" );
+       return ();
+    } elsif( $args{'Object'} ) {
+       my $obj = $args{'Object'};
+       return ($obj, ref $obj, $obj->id);
+    } elsif ( $args{'ObjectType'} ) {
+       my $obj =  $args{'ObjectType'}->new( $self->CurrentUser );
+       $obj->Load( $args{'ObjectId'} );
+       return ($obj, ref $obj, $obj->id);
+    } else {
+       $RT::Logger->crit( "Method called with wrong args" );
+       return ();
+    }
+}
+
+
 # }}}
 1;