[Rt-commit] rt branch, remove-delegation, updated. rt-3.8.8-726-g4564359

Jesse Vincent jesse at bestpractical.com
Fri Sep 17 21:37:47 EDT 2010


The branch, remove-delegation has been updated
       via  45643594eb5ca07184ad02d34943610b4c9034f0 (commit)
      from  69f7b530b88eefe3bafe3fcc3b5daa5afee35a85 (commit)

Summary of changes:
 UPGRADING                 |    2 +-
 etc/upgrade/3.9.2/content |   52 +++++++++++------
 lib/RT/Group_Overlay.pm   |  140 ++-------------------------------------------
 lib/RT/Groups_Overlay.pm  |   23 -------
 lib/RT/System.pm          |    6 --
 5 files changed, 40 insertions(+), 183 deletions(-)

- Log -----------------------------------------------------------------
commit 45643594eb5ca07184ad02d34943610b4c9034f0
Author: Jesse Vincent <jesse at bestpractical.com>
Date:   Fri Sep 17 19:02:56 2010 -0400

    First pass at removing "personal groups" feature. Needs testing,
    especially the upgrade feature.

diff --git a/UPGRADING b/UPGRADING
index fd7dc4f..e34b789 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -27,7 +27,7 @@ The deprecated classes RT::Action::Generic, RT::Condition::Generic and RT::Searc
 have been removed, but you shouldn't have been using them anyway. You should have been using
 RT::Action, RT::Condition and RT::Search, respectively.
 
-* The "Rights Delegation" feature has been removed.
+* The "Rights Delegation" and "Personal Groups" features have been removed.
 
 *******
 UPGRADING FROM 3.8.8 and earlier - Changes:
diff --git a/etc/upgrade/3.9.2/content b/etc/upgrade/3.9.2/content
index 9f7339e..bb06cce 100644
--- a/etc/upgrade/3.9.2/content
+++ b/etc/upgrade/3.9.2/content
@@ -3,28 +3,44 @@
         use strict;
         $RT::Logger->debug('Removing all delegated rights');
 
-
         my $acl = RT::ACL->new($RT::SystemUser);
-        $acl->Limit(
-            CLAUSE          => 'search',
-            FIELD           => 'DelegatedBy',
-            OPERATOR        => '>',
-            VALUE           => '0'
-        );
-        $acl->Limit(
-            CLAUSE          => 'search',
-            FIELD           => 'DelegatedFrom',
-            OPERATOR        => '>',
-            VALUE           => '0',
-            ENTRYAGGREGATOR => 'OR',
-        );
+        $acl->Limit( CLAUSE   => 'search',
+                     FIELD    => 'DelegatedBy',
+                     OPERATOR => '>',
+                     VALUE    => '0'
+                   );
+        $acl->Limit( CLAUSE          => 'search',
+                     FIELD           => 'DelegatedFrom',
+                     OPERATOR        => '>',
+                     VALUE           => '0',
+                     ENTRYAGGREGATOR => 'OR',
+                   );
+
+        while ( my $ace = $acl->Next ) {
+            my ( $ok, $msg ) = $ace->Delete();
 
-        while (my $ace = $acl->Next) {
-            my ($ok, $msg) = $ace->Delete();
+            if ( !$ok ) {
+                $RT::Logger->warn(
+                           "Unable to delete ACE " . $ace->id . ": " . $msg );
+            }
+        }
 
-            if (!$ok) {
-                $RT::Logger->warn("Unable to delete ACE ".$ace->id.": ".$msg);
+        my $groups = RT::Groups->new($RT::SystemUser);
+        $groups->Limit( FIELD    => 'Domain',
+                        OPERATOR => '=',
+                        VALUE    => 'Personal'
+                      );
+        while ( my $group = $groups->Next ) {
+            my $members = $group->MembersObj();
+            while ( my $member = $membersObj->Next ) {
+                my ( $ok, $msg ) = $group->DeleteMember( $member->MemberId );
+                if ( !$ok ) {
+                    $RT::Logger->warn(   "Unable to remove group member "
+                                       . $member->id . ": "
+                                       . $msg );
+                }
             }
+            $group->RT::Record::Delete();
         }
     },
 );
diff --git a/lib/RT/Group_Overlay.pm b/lib/RT/Group_Overlay.pm
index 88f572d..ac26663 100755
--- a/lib/RT/Group_Overlay.pm
+++ b/lib/RT/Group_Overlay.pm
@@ -192,11 +192,6 @@ sub SelfDescription {
 	elsif ($self->Domain eq 'UserDefined') {
 		return $self->loc("group '[_1]'",$self->Name);
 	}
-	elsif ($self->Domain eq 'Personal') {
-		my $user = RT::User->new($self->CurrentUser);
-		$user->Load($self->Instance);
-		return $self->loc("personal group '[_1]' for user '[_2]'",$self->Name, $user->Name);
-	}
 	elsif ($self->Domain eq 'RT::System-Role') {
 		return $self->loc("system [_1]",$self->Type);
 	}
@@ -299,27 +294,6 @@ sub LoadACLEquivalenceGroup {
 
 # }}}
 
-# {{{ sub LoadPersonalGroup 
-
-=head2 LoadPersonalGroup {Name => NAME, User => USERID}
-
-Loads a personal group from the database. 
-
-=cut
-
-sub LoadPersonalGroup {
-    my $self       = shift;
-    my %args =  (   Name => undef,
-                    User => undef,
-                    @_);
-
-        $self->LoadByCols( "Domain" => 'Personal',
-                           "Instance" => $args{'User'},
-                           "Type" => '',
-                           "Name" => $args{'Name'} );
-}
-
-# }}}
 
 # {{{ sub LoadSystemInternalGroup 
 
@@ -583,57 +557,6 @@ sub _CreateACLEquivalenceGroup {
 
 # }}}
 
-# {{{ CreatePersonalGroup
-
-=head2 CreatePersonalGroup { PrincipalId => PRINCIPAL_ID, Name => "name", Description => "Description"}
-
-A helper subroutine which creates a personal group. Generally,
-personal groups are used for ACL delegation and adding to ticket roles
-PrincipalId defaults to the current user's principal id.
-
-Returns a tuple of (Id, Message).  If id is 0, the create failed
-
-=cut
-
-sub CreatePersonalGroup {
-    my $self = shift;
-    my %args = (
-        Name        => undef,
-        Description => undef,
-        PrincipalId => $self->CurrentUser->PrincipalId,
-        @_
-    );
-
-    if ( $self->CurrentUser->PrincipalId == $args{'PrincipalId'} ) {
-
-        unless ( $self->CurrentUserHasRight('AdminOwnPersonalGroups') ) {
-            $RT::Logger->warning( $self->CurrentUser->Name
-                  . " Tried to create a group without permission." );
-            return ( 0, $self->loc('Permission Denied') );
-        }
-
-    }
-    else {
-        unless ( $self->CurrentUserHasRight('AdminAllPersonalGroups') ) {
-            $RT::Logger->warning( $self->CurrentUser->Name
-                  . " Tried to create a group without permission." );
-            return ( 0, $self->loc('Permission Denied') );
-        }
-
-    }
-
-    return (
-        $self->_Create(
-            Domain      => 'Personal',
-            Type        => '',
-            Instance    => $args{'PrincipalId'},
-            Name        => $args{'Name'},
-            Description => $args{'Description'}
-        )
-    );
-}
-
-# }}}
 
 # {{{ CreateRoleGroup 
 
@@ -713,21 +636,8 @@ This routine finds all the cached group members that are members of this group
  sub SetDisabled {
      my $self = shift;
      my $val = shift;
-    if ($self->Domain eq 'Personal') {
-   		if ($self->CurrentUser->PrincipalId == $self->Instance) {
-    		unless ( $self->CurrentUserHasRight('AdminOwnPersonalGroups')) {
-        		return ( 0, $self->loc('Permission Denied') );
-    		}
-    	} else {
-        	unless ( $self->CurrentUserHasRight('AdminAllPersonalGroups') ) {
-   	    		 return ( 0, $self->loc('Permission Denied') );
-    		}
-    	}
-	}
-	else {
-        unless ( $self->CurrentUserHasRight('AdminGroup') ) {
-                 return (0, $self->loc('Permission Denied'));
-    }
+     unless ( $self->CurrentUserHasRight('AdminGroup') ) {
+        return (0, $self->loc('Permission Denied'));
     }
     $RT::Handle->BeginTransaction();
     $self->PrincipalObj->SetDisabled($val);
@@ -838,7 +748,7 @@ By default returns groups including all subgroups, but
 could be changed with C<Recursively> named argument.
 
 B<Note> that groups are not filtered by type and result
-may contain as well system groups, personal and other.
+may contain as well system groups and others.
 
 =cut
 
@@ -968,19 +878,6 @@ sub AddMember {
 
 
 
-    if ($self->Domain eq 'Personal') {
-   		if ($self->CurrentUser->PrincipalId == $self->Instance) {
-    		unless ( $self->CurrentUserHasRight('AdminOwnPersonalGroups')) {
-        		return ( 0, $self->loc('Permission Denied') );
-    		}
-    	} else {
-        	unless ( $self->CurrentUserHasRight('AdminAllPersonalGroups') ) {
-   	    		 return ( 0, $self->loc('Permission Denied') );
-    		}
-    	}
-	}
-	
-	else {	
     # We should only allow membership changes if the user has the right 
     # to modify group membership or the user is the principal in question
     # and the user has the right to modify his own membership
@@ -991,7 +888,6 @@ sub AddMember {
         return ( 0, $self->loc("Permission Denied") );
     }
 
-  	} 
     $self->_AddMember(PrincipalId => $new_member);
 }
 
@@ -1166,25 +1062,12 @@ sub DeleteMember {
     # to modify group membership or the user is the principal in question
     # and the user has the right to modify his own membership
 
-    if ($self->Domain eq 'Personal') {
-   		if ($self->CurrentUser->PrincipalId == $self->Instance) {
-    		unless ( $self->CurrentUserHasRight('AdminOwnPersonalGroups')) {
-        		return ( 0, $self->loc('Permission Denied') );
-    		}
-    	} else {
-        	unless ( $self->CurrentUserHasRight('AdminAllPersonalGroups') ) {
-   	    		 return ( 0, $self->loc('Permission Denied') );
-    		}
-    	}
-	}
-	else {
     unless ( (($member_id == $self->CurrentUser->PrincipalId) &&
 	      $self->CurrentUserHasRight('ModifyOwnMembership') ) ||
 	      $self->CurrentUserHasRight('AdminGroupMembership') ) {
         #User has no permission to be doing this
         return ( 0, $self->loc("Permission Denied") );
     }
-	}
     $self->_DeleteMember($member_id);
 }
 
@@ -1237,21 +1120,8 @@ sub _Set {
         @_
     );
 
-	if ($self->Domain eq 'Personal') {
-   		if ($self->CurrentUser->PrincipalId == $self->Instance) {
-    		unless ( $self->CurrentUserHasRight('AdminOwnPersonalGroups')) {
-        		return ( 0, $self->loc('Permission Denied') );
-    		}
-    	} else {
-        	unless ( $self->CurrentUserHasRight('AdminAllPersonalGroups') ) {
-   	    		 return ( 0, $self->loc('Permission Denied') );
-    		}
-    	}
-	}
-	else {
-    	unless ( $self->CurrentUserHasRight('AdminGroup') ) {
-        	return ( 0, $self->loc('Permission Denied') );
-    	}
+    unless ( $self->CurrentUserHasRight('AdminGroup') ) {
+      	return ( 0, $self->loc('Permission Denied') );
 	}
 
     my $Old = $self->SUPER::_Value("$args{'Field'}");
diff --git a/lib/RT/Groups_Overlay.pm b/lib/RT/Groups_Overlay.pm
index 4774ae2..ba86bbd 100755
--- a/lib/RT/Groups_Overlay.pm
+++ b/lib/RT/Groups_Overlay.pm
@@ -168,29 +168,6 @@ sub LimitToUserDefinedGroups {
 
 # }}}
 
-# {{{ LimitToPersonalGroupsFor
-
-=head2 LimitToPersonalGroupsFor PRINCIPAL_ID
-
-Return only Personal Groups for the user whose principal id 
-is PRINCIPAL_ID
-
-=cut
-
-
-sub LimitToPersonalGroupsFor {
-    my $self = shift;
-    my $princ = shift;
-
-    $self->Limit(FIELD => 'Domain', OPERATOR => '=', VALUE => 'Personal');
-    $self->Limit(   FIELD => 'Instance',   
-                    OPERATOR => '=', 
-                    VALUE => $princ);
-}
-
-
-# }}}
-
 # {{{ LimitToRolesForQueue
 
 =head2 LimitToRolesForQueue QUEUE_ID
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index 1927ccc..87342e4 100755
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -77,10 +77,6 @@ use RT::ACL;
 # XXX TODO Can't localize these outside of having an object around.
 our $RIGHTS = {
     SuperUser              => 'Do anything and everything',           # loc_pair
-    AdminAllPersonalGroups =>
-      "Create, delete and modify the members of any user's personal groups", # loc_pair
-    AdminOwnPersonalGroups =>
-      'Create, delete and modify the members of personal groups',     # loc_pair
     AdminUsers     => 'Create, delete and modify users',              # loc_pair
     ModifySelf     => "Modify one's own RT account",                  # loc_pair
     ShowConfigTab => "Show Configuration tab",     # loc_pair
@@ -93,8 +89,6 @@ our $RIGHTS = {
 
 our $RIGHT_CATEGORIES = {
     SuperUser              => 'Admin',
-    AdminAllPersonalGroups => 'Admin',
-    AdminOwnPersonalGroups => 'Admin',
     AdminUsers             => 'Admin',
     ModifySelf             => 'Staff',
     ShowConfigTab          => 'Admin',

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


More information about the Rt-commit mailing list