import rt 3.8.10

[freeside.git] / rt / lib / RT / Users_Overlay.pm

diff --git a/rt/lib/RT/Users_Overlay.pm b/rt/lib/RT/Users_Overlay.pm
index 4bb9f8f..9640925 100644 (file)
--- a/rt/lib/RT/Users_Overlay.pm
+++ b/rt/lib/RT/Users_Overlay.pm
@@ -1,38 +1,40 @@
 # BEGIN BPS TAGGED BLOCK {{{
 # BEGIN BPS TAGGED BLOCK {{{

-# 
+#

 # COPYRIGHT:
 # COPYRIGHT:

-#  
-# This software is Copyright (c) 1996-2005 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)
 # (Except where explicitly superseded by other copyright notices)

-# 
-# 
+#
+#

 # LICENSE:
 # 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 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.
 # 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
 # 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:
 # 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.)
 # (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
 # 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,7 +43,7 @@
 # royalty-free, perpetual, license to use, copy, create derivative
 # works based on those contributions, and sublicense and distribute
 # those contributions and any derivatives thereof.
 # 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 }}}
 
 =head1 NAME
 # END BPS TAGGED BLOCK }}}
 
 =head1 NAME


@@ -58,11 +60,6 @@
 
 =head1 METHODS
 
 
 =head1 METHODS
 

-=begin testing
-
-ok(require RT::Users);
-
-=end testing

 
 =cut
 
 
 =cut
 


@@ -76,11 +73,10 @@ no warnings qw(redefine);
 sub _Init {
     my $self = shift;
     $self->{'table'} = 'Users';
 sub _Init {
     my $self = shift;
     $self->{'table'} = 'Users';

-        $self->{'primary_key'} = 'id';
-
+    $self->{'primary_key'} = 'id';
+    $self->{'with_disabled_column'} = 1;

 
 

-
-    my @result =          $self->SUPER::_Init(@_);
+    my @result = $self->SUPER::_Init(@_);

     # By default, order by name
     $self->OrderBy( ALIAS => 'main',
                     FIELD => 'Name',
     # By default, order by name
     $self->OrderBy( ALIAS => 'main',
                     FIELD => 'Name',


@@ -88,10 +84,15 @@ sub _Init {
 
     $self->{'princalias'} = $self->NewAlias('Principals');
 
 
     $self->{'princalias'} = $self->NewAlias('Principals');
 

+    # XXX: should be generalized

     $self->Join( ALIAS1 => 'main',
                  FIELD1 => 'id',
                  ALIAS2 => $self->{'princalias'},
                  FIELD2 => 'id' );
     $self->Join( ALIAS1 => 'main',
                  FIELD1 => 'id',
                  ALIAS2 => $self->{'princalias'},
                  FIELD2 => 'id' );

+    $self->Limit( ALIAS => $self->{'princalias'},
+                  FIELD => 'PrincipalType',
+                  VALUE => 'User',
+                );

 
     return (@result);
 }
 
     return (@result);
 }


@@ -102,9 +103,9 @@ sub _Init {
 
 Returns the string that represents this Users object's primary "Principals" alias.
 
 
 Returns the string that represents this Users object's primary "Principals" alias.
 

-

 =cut
 
 =cut
 

+# XXX: should be generalized

 sub PrincipalsAlias {
     my $self = shift;
     return($self->{'princalias'});
 sub PrincipalsAlias {
     my $self = shift;
     return($self->{'princalias'});


@@ -112,45 +113,41 @@ sub PrincipalsAlias {
 }
 
 
 }
 
 

-# {{{ sub _DoSearch 
-
-=head2 _DoSearch
+=head2 LimitToEnabled

 
 

-  A subclass of DBIx::SearchBuilder::_DoSearch that makes sure that _Disabled rows never get seen unless
-we're explicitly trying to see them.
+Only find items that haven\'t been disabled

 
 =cut
 
 
 =cut
 

-sub _DoSearch {
+# XXX: should be generalized
+sub LimitToEnabled {

     my $self = shift;
 
     my $self = shift;
 

-    #unless we really want to find disabled rows, make sure we\'re only finding enabled ones.
-    unless ( $self->{'find_disabled_rows'} ) {
-        $self->LimitToEnabled();
-    }
-    return ( $self->SUPER::_DoSearch(@_) );
-
+    $self->{'handled_disabled_column'} = 1;
+    $self->Limit(
+        ALIAS    => $self->PrincipalsAlias,
+        FIELD    => 'Disabled',
+        VALUE    => '0',
+    );

 }
 
 }
 

-# }}}
-# {{{ sub LimitToEnabled
-
-=head2 LimitToEnabled
+=head2 LimitToDeleted

 
 

-Only find items that haven\'t been disabled
+Only find items that have been deleted.

 
 =cut
 
 
 =cut
 

-sub LimitToEnabled {
+sub LimitToDeleted {

     my $self = shift;
     my $self = shift;

-
-    $self->Limit( ALIAS    => $self->{'princalias'},
-                  FIELD    => 'Disabled',
-                  VALUE    => '0',
-                  OPERATOR => '=' );
+    
+    $self->{'handled_disabled_column'} = $self->{'find_disabled_rows'} = 1;
+    $self->Limit(
+        ALIAS => $self->PrincipalsAlias,
+        FIELD => 'Disabled',
+        VALUE => 1,
+    );

 }
 
 }
 

-# }}}

 
 # {{{ LimitToEmail
 
 
 # {{{ LimitToEmail
 


@@ -187,7 +184,7 @@ sub MemberOfGroup {
     my $groupalias = $self->NewAlias('CachedGroupMembers');
 
     # Join the principal to the groups table
     my $groupalias = $self->NewAlias('CachedGroupMembers');
 
     # Join the principal to the groups table

-    $self->Join( ALIAS1 => $self->{'princalias'},
+    $self->Join( ALIAS1 => $self->PrincipalsAlias,

                  FIELD1 => 'id',
                  ALIAS2 => $groupalias,
                  FIELD2 => 'MemberId' );
                  FIELD1 => 'id',
                  ALIAS2 => $groupalias,
                  FIELD2 => 'MemberId' );


@@ -225,132 +222,86 @@ sub LimitToPrivileged {
 
 =head2 WhoHaveRight { Right => 'name', Object => $rt_object , IncludeSuperusers => undef, IncludeSubgroupMembers => undef, IncludeSystemRights => undef, EquivObjects => [ ] }
 
 
 =head2 WhoHaveRight { Right => 'name', Object => $rt_object , IncludeSuperusers => undef, IncludeSubgroupMembers => undef, IncludeSystemRights => undef, EquivObjects => [ ] }
 

-=begin testing
-
-ok(my $users = RT::Users->new($RT::SystemUser));
-$users->WhoHaveRight(Object =>$RT::System, Right =>'SuperUser');
-ok($users->Count == 1, "There is one privileged superuser - Found ". $users->Count );
-# TODO: this wants more testing
-
-my $RTxUser = RT::User->new($RT::SystemUser);
-($id, $msg) = $RTxUser->Create( Name => 'RTxUser', Comments => "RTx extension user", Privileged => 1);
-ok ($id,$msg);
-
-my $group = RT::Group->new($RT::SystemUser);
-$group->LoadACLEquivalenceGroup($RTxUser->PrincipalObj);
-
-my $RTxSysObj = {};
-bless $RTxSysObj, 'RTx::System';
-*RTx::System::Id = sub { 1; };
-*RTx::System::id = *RTx::System::Id;
-my $ace = RT::Record->new($RT::SystemUser);
-$ace->Table('ACL');
-$ace->_BuildTableAttributes unless ($_TABLE_ATTR->{ref($self)});
-($id, $msg) = $ace->Create( PrincipalId => $group->id, PrincipalType => 'Group', RightName => 'RTxUserRight', ObjectType => 'RTx::System', ObjectId  => 1 );
-ok ($id, "ACL for RTxSysObj created");
-
-my $RTxObj = {};
-bless $RTxObj, 'RTx::System::Record';
-*RTx::System::Record::Id = sub { 4; };
-*RTx::System::Record::id = *RTx::System::Record::Id;
-
-$users = RT::Users->new($RT::SystemUser);
-$users->WhoHaveRight(Right => 'RTxUserRight', Object => $RTxSysObj);
-is($users->Count, 1, "RTxUserRight found for RTxSysObj");
-
-$users = RT::Users->new($RT::SystemUser);
-$users->WhoHaveRight(Right => 'RTxUserRight', Object => $RTxObj);
-is($users->Count, 0, "RTxUserRight not found for RTxObj");
-
-$users = RT::Users->new($RT::SystemUser);
-$users->WhoHaveRight(Right => 'RTxUserRight', Object => $RTxObj, EquivObjects => [ $RTxSysObj ]);
-is($users->Count, 1, "RTxUserRight found for RTxObj using EquivObjects");
-
-$ace = RT::Record->new($RT::SystemUser);
-$ace->Table('ACL');
-$ace->_BuildTableAttributes unless ($_TABLE_ATTR->{ref($self)});
-($id, $msg) = $ace->Create( PrincipalId => $group->id, PrincipalType => 'Group', RightName => 'RTxUserRight', ObjectType => 'RTx::System::Record', ObjectId => 5 );
-ok ($id, "ACL for RTxObj created");
-
-my $RTxObj2 = {};
-bless $RTxObj2, 'RTx::System::Record';
-*RTx::System::Record::Id = sub { 5; };
-*RTx::System::Record::id = sub { 5; };
-
-$users = RT::Users->new($RT::SystemUser);
-$users->WhoHaveRight(Right => 'RTxUserRight', Object => $RTxObj2);
-is($users->Count, 1, "RTxUserRight found for RTxObj2");
-
-$users = RT::Users->new($RT::SystemUser);
-$users->WhoHaveRight(Right => 'RTxUserRight', Object => $RTxObj2, EquivObjects => [ $RTxSysObj ]);
-is($users->Count, 1, "RTxUserRight found for RTxObj2");
-
-
-=end testing
-

 
 find all users who the right Right for this group, either individually
 or as members of groups
 
 
 find all users who the right Right for this group, either individually
 or as members of groups
 

-

 If passed a queue object, with no id, it will find users who have that right for _any_ queue
 
 If passed a queue object, with no id, it will find users who have that right for _any_ queue
 

-
-

 =cut
 
 =cut
 

-sub WhoHaveRight {
+# XXX: should be generalized
+sub _JoinGroupMembers
+{

     my $self = shift;
     my %args = (
     my $self = shift;
     my %args = (

-        Right                  => undef,
-        Object                 => undef,
-        IncludeSystemRights    => undef,
-        IncludeSuperusers      => undef,

         IncludeSubgroupMembers => 1,
         IncludeSubgroupMembers => 1,

-        EquivObjects           => [ ],

         @_
     );
 
         @_
     );
 

-    if ( defined $args{'ObjectType'} || defined $args{'ObjectId'} ) {
-        $RT::Logger->crit( "$self WhoHaveRight called with the Obsolete ObjectId/ObjectType API");
-        return (undef);
-    }
-    
-
-    # Find only members of groups that have the right.
-
-    my $acl       = $self->NewAlias('ACL');
-    my $groups    = $self->NewAlias('Groups');
-    my $userprinc = $self->{'princalias'};
-
-# The cachedgroupmembers table is used for unrolling group memberships to allow fast lookups
-# if we bind to CachedGroupMembers, we'll find all members of groups recursively.
-# if we don't we'll find only 'direct' members of the group in question
-    my $cgm;
+    my $principals = $self->PrincipalsAlias;

 
 

+    # The cachedgroupmembers table is used for unrolling group memberships
+    # to allow fast lookups. if we bind to CachedGroupMembers, we'll find
+    # all members of groups recursively. if we don't we'll find only 'direct'
+    # members of the group in question
+    my $group_members;

     if ( $args{'IncludeSubgroupMembers'} ) {
     if ( $args{'IncludeSubgroupMembers'} ) {

-        $cgm = $self->NewAlias('CachedGroupMembers');
+        $group_members = $self->NewAlias('CachedGroupMembers');

     }
     else {
     }
     else {

-        $cgm = $self->NewAlias('GroupMembers');
+        $group_members = $self->NewAlias('GroupMembers');

     }
 
     }
 

-#Tie the users we're returning ($userprinc) to the groups that have rights granted to them ($groupprinc)

     $self->Join(
     $self->Join(

-        ALIAS1 => $cgm,
+        ALIAS1 => $group_members,

         FIELD1 => 'MemberId',
         FIELD1 => 'MemberId',

-        ALIAS2 => $userprinc,
+        ALIAS2 => $principals,

         FIELD2 => 'id'
     );
 
         FIELD2 => 'id'
     );
 

+    return $group_members;
+}
+
+# XXX: should be generalized
+sub _JoinGroups
+{
+    my $self = shift;
+    my %args = (@_);
+
+    my $group_members = $self->_JoinGroupMembers( %args );
+    my $groups = $self->NewAlias('Groups');

     $self->Join(
         ALIAS1 => $groups,
         FIELD1 => 'id',
     $self->Join(
         ALIAS1 => $groups,
         FIELD1 => 'id',

-        ALIAS2 => $cgm,
+        ALIAS2 => $group_members,

         FIELD2 => 'GroupId'
     );
 
         FIELD2 => 'GroupId'
     );
 

-# {{{ Find only rows where the right granted is the one we're looking up or _possibly_ superuser
+    return $groups;
+}
+
+# XXX: should be generalized
+sub _JoinACL
+{
+    my $self = shift;
+    my %args = (
+        Right                  => undef,
+        IncludeSuperusers      => undef,
+        @_,
+    );
+
+    if ( $args{'Right'} ) {
+        my $canonic = RT::ACE->CanonicalizeRightName( $args{'Right'} );
+        unless ( $canonic ) {
+            $RT::Logger->error("Invalid right. Couldn't canonicalize right '$args{'Right'}'");
+        }
+        else {
+            $args{'Right'} = $canonic;
+        }
+    }
+
+    my $acl = $self->NewAlias('ACL');

     $self->Limit(
         ALIAS    => $acl,
         FIELD    => 'RightName',
     $self->Limit(
         ALIAS    => $acl,
         FIELD    => 'RightName',


@@ -358,7 +309,6 @@ sub WhoHaveRight {
         VALUE => $args{Right} || 'NULL',
         ENTRYAGGREGATOR => 'OR'
     );
         VALUE => $args{Right} || 'NULL',
         ENTRYAGGREGATOR => 'OR'
     );

-

     if ( $args{'IncludeSuperusers'} and $args{'Right'} ) {
         $self->Limit(
             ALIAS           => $acl,
     if ( $args{'IncludeSuperusers'} and $args{'Right'} ) {
         $self->Limit(
             ALIAS           => $acl,


@@ -368,97 +318,240 @@ sub WhoHaveRight {
             ENTRYAGGREGATOR => 'OR'
         );
     }
             ENTRYAGGREGATOR => 'OR'
         );
     }

+    return $acl;
+}
+
+# XXX: should be generalized
+sub _GetEquivObjects
+{
+    my $self = shift;
+    my %args = (
+        Object                 => undef,
+        IncludeSystemRights    => undef,
+        EquivObjects           => [ ],
+        @_
+    );
+    return () unless $args{'Object'};
+
+    my @objects = ($args{'Object'});
+    if ( UNIVERSAL::isa( $args{'Object'}, 'RT::Ticket' ) ) {
+        # If we're looking at ticket rights, we also want to look at the associated queue rights.
+        # this is a little bit hacky, but basically, now that we've done the ticket roles magic,
+        # we load the queue object and ask all the rest of our questions about the queue.
+
+        # XXX: This should be abstracted into object itself
+        if( $args{'Object'}->id ) {
+            push @objects, $args{'Object'}->ACLEquivalenceObjects;
+        } else {
+            push @objects, 'RT::Queue';
+        }
+    }

 
 

-    # }}}
+    if( $args{'IncludeSystemRights'} ) {
+        push @objects, 'RT::System';
+    }
+    push @objects, @{ $args{'EquivObjects'} };
+    return grep $_, @objects;
+}

 
 

-    my ( $or_check_ticket_roles, $or_check_roles );
-    my $which_object = "$acl.ObjectType = 'RT::System'";
+# XXX: should be generalized
+sub WhoHaveRight {
+    my $self = shift;
+    my %args = (
+        Right                  => undef,
+        Object                 => undef,
+        IncludeSystemRights    => undef,
+        IncludeSuperusers      => undef,
+        IncludeSubgroupMembers => 1,
+        EquivObjects           => [ ],
+        @_
+    );

 
 

-    if ( defined $args{'Object'} ) {
-        if ( ref( $args{'Object'} ) eq 'RT::Ticket' ) {
-            $or_check_ticket_roles = " OR ( $groups.Domain = 'RT::Ticket-Role' AND $groups.Instance = " . $args{'Object'}->Id . ") ";
+    if ( defined $args{'ObjectType'} || defined $args{'ObjectId'} ) {
+        $RT::Logger->crit( "WhoHaveRight called with the Obsolete ObjectId/ObjectType API");
+        return (undef);
+    }

 
 

-# If we're looking at ticket rights, we also want to look at the associated queue rights.
-# this is a little bit hacky, but basically, now that we've done the ticket roles magic,
-# we load the queue object and ask all the rest of our questions about the queue.
-            $args{'Object'} = $args{'Object'}->QueueObj;
-        }
+    my $from_role = $self->Clone;
+    $from_role->WhoHaveRoleRight( %args );

 
 

-        # TODO XXX This really wants some refactoring
-        if ( ref( $args{'Object'} ) eq 'RT::Queue' ) {
-            $or_check_roles = " OR ( ( ($groups.Domain = 'RT::Queue-Role' ";
-            $or_check_roles .= "AND $groups.Instance = " . $args{'Object'}->id if ( $args{'Object'}->id );
-            $or_check_roles .= ") $or_check_ticket_roles ) " . " AND $groups.Type = $acl.PrincipalType) ";
-        }
-        if ( $args{'IncludeSystemRights'} ) {
-            $which_object .= ' OR ';
-        }
-        else {
-            $which_object = '';
-        }
-        foreach my $obj ( @{ $args{'EquivObjects'} } ) {
-            $which_object .= "($acl.ObjectType = '" . ref( $obj ) . "' AND $acl.ObjectId = " . $obj->id . ") OR ";
-        }
-        $which_object .= " ($acl.ObjectType = '" . ref( $args{'Object'} ) . "'";
-        if ( $args{'Object'}->id ) {
-            $which_object .= " AND $acl.ObjectId = " . $args{'Object'}->id;
-        }
+    my $from_group = $self->Clone;
+    $from_group->WhoHaveGroupRight( %args );

 
 

-        $which_object .=  ") ";
-    }
-    $self->_AddSubClause( "WhichObject", "($which_object)" );
-    $self->_AddSubClause(
-        "WhichGroup",
-            qq{ ( (    $acl.PrincipalId = $groups.id AND $acl.PrincipalType = 'Group' 
-                AND (   $groups.Domain = 'SystemInternal' OR $groups.Domain = 'UserDefined' OR $groups.Domain = 'ACLEquivalence')) 
-                $or_check_roles) }
+    #XXX: DIRTY HACK
+    use DBIx::SearchBuilder::Union;
+    my $union = new DBIx::SearchBuilder::Union;
+    $union->add( $from_group );
+    $union->add( $from_role );
+    %$self = %$union;
+    bless $self, ref($union);
+
+    return;
+}
+# }}}
+
+# XXX: should be generalized
+sub WhoHaveRoleRight
+{
+    my $self = shift;
+    my %args = (
+        Right                  => undef,
+        Object                 => undef,
+        IncludeSystemRights    => undef,
+        IncludeSuperusers      => undef,
+        IncludeSubgroupMembers => 1,
+        EquivObjects           => [ ],
+        @_

     );
     );

-    # only include regular RT users
-    $self->LimitToEnabled;
+
+    my @objects = $self->_GetEquivObjects( %args );
+
+    # RT::Principal->RolesWithRight only expects EquivObjects, so we need to
+    # fill it.  At the very least it needs $args{Object}, which
+    # _GetEquivObjects above does for us.
+    unshift @{$args{'EquivObjects'}}, @objects;
+
+    my @roles = RT::Principal->RolesWithRight( %args );
+    unless ( @roles ) {
+        $self->_AddSubClause( "WhichRole", "(main.id = 0)" );
+        return;
+    }
+
+    my $groups = $self->_JoinGroups( %args );

 
     # no system user
 
     # no system user

-    $self->Limit( ALIAS => $userprinc, FIELD => 'id', OPERATOR => '!=', VALUE => $RT::SystemUser->id);
+    $self->Limit( ALIAS => $self->PrincipalsAlias,
+                  FIELD => 'id',
+                  OPERATOR => '!=',
+                  VALUE => $RT::SystemUser->id
+                );

 
 

+    $self->_AddSubClause( "WhichRole", "(". join( ' OR ', map "$groups.Type = '$_'", @roles ) .")" );
+
+    my @groups_clauses = $self->_RoleClauses( $groups, @objects );
+    $self->_AddSubClause( "WhichObject", "(". join( ' OR ', @groups_clauses ) .")" )
+        if @groups_clauses;
+
+    return;

 }
 }

-# }}}

 
 

-# {{{ WhoBelongToGroups 
+sub _RoleClauses {
+    my $self = shift;
+    my $groups = shift;
+    my @objects = @_;
+
+    my @groups_clauses;
+    foreach my $obj ( @objects ) {
+        my $type = ref($obj)? ref($obj): $obj;
+        my $id;
+        $id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
+
+        my $role_clause = "$groups.Domain = '$type-Role'";
+        # XXX: Groups.Instance is VARCHAR in DB, we should quote value
+        # if we want mysql 4.0 use indexes here. we MUST convert that
+        # field to integer and drop this quotes.
+        $role_clause   .= " AND $groups.Instance = '$id'" if $id;
+        push @groups_clauses, "($role_clause)";
+    }
+    return @groups_clauses;
+}
+
+# XXX: should be generalized
+sub _JoinGroupMembersForGroupRights
+{
+    my $self = shift;
+    my %args = (@_);
+    my $group_members = $self->_JoinGroupMembers( %args );
+    $self->Limit( ALIAS => $args{'ACLAlias'},
+                  FIELD => 'PrincipalId',
+                  VALUE => "$group_members.GroupId",
+                  QUOTEVALUE => 0,
+                );
+}
+
+# XXX: should be generalized
+sub WhoHaveGroupRight
+{
+    my $self = shift;
+    my %args = (
+        Right                  => undef,
+        Object                 => undef,
+        IncludeSystemRights    => undef,
+        IncludeSuperusers      => undef,
+        IncludeSubgroupMembers => 1,
+        EquivObjects           => [ ],
+        @_
+    );
+
+    # Find only rows where the right granted is
+    # the one we're looking up or _possibly_ superuser
+    my $acl = $self->_JoinACL( %args );
+
+    my ($check_objects) = ('');
+    my @objects = $self->_GetEquivObjects( %args );
+
+    if ( @objects ) {
+        my @object_clauses;
+        foreach my $obj ( @objects ) {
+            my $type = ref($obj)? ref($obj): $obj;
+            my $id;
+            $id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
+
+            my $object_clause = "$acl.ObjectType = '$type'";
+            $object_clause   .= " AND $acl.ObjectId   = $id" if $id;
+            push @object_clauses, "($object_clause)";
+        }
+
+        $check_objects = join ' OR ', @object_clauses;
+    } else {
+        if( !$args{'IncludeSystemRights'} ) {
+            $check_objects = "($acl.ObjectType != 'RT::System')";
+        }
+    }
+    $self->_AddSubClause( "WhichObject", "($check_objects)" );
+    
+    $self->_JoinGroupMembersForGroupRights( %args, ACLAlias => $acl );
+    # Find only members of groups that have the right.
+    $self->Limit( ALIAS => $acl,
+                  FIELD => 'PrincipalType',
+                  VALUE => 'Group',
+                );
+    
+    # no system user
+    $self->Limit( ALIAS => $self->PrincipalsAlias,
+                  FIELD => 'id',
+                  OPERATOR => '!=',
+                  VALUE => $RT::SystemUser->id
+                );
+    return;
+}
+
+# {{{ WhoBelongToGroups

 
 =head2 WhoBelongToGroups { Groups => ARRAYREF, IncludeSubgroupMembers => 1 }
 
 =cut
 
 
 =head2 WhoBelongToGroups { Groups => ARRAYREF, IncludeSubgroupMembers => 1 }
 
 =cut
 

+# XXX: should be generalized

 sub WhoBelongToGroups {
     my $self = shift;
     my %args = ( Groups                 => undef,
                  IncludeSubgroupMembers => 1,
                  @_ );
 
 sub WhoBelongToGroups {
     my $self = shift;
     my %args = ( Groups                 => undef,
                  IncludeSubgroupMembers => 1,
                  @_ );
 

-    # Unprivileged users can't be granted real system rights. 
+    # Unprivileged users can't be granted real system rights.

     # is this really the right thing to be saying?
     $self->LimitToPrivileged();
 
     # is this really the right thing to be saying?
     $self->LimitToPrivileged();
 

-    my $userprinc  = $self->{'princalias'};
-    my $cgm;
-
-    # The cachedgroupmembers table is used for unrolling group memberships to allow fast lookups 
-    # if we bind to CachedGroupMembers, we'll find all members of groups recursively.
-    # if we don't we'll find only 'direct' members of the group in question
-
-    if ( $args{'IncludeSubgroupMembers'} ) {
-        $cgm = $self->NewAlias('CachedGroupMembers');
-    }
-    else {
-        $cgm = $self->NewAlias('GroupMembers');
-    }
-
-    #Tie the users we're returning ($userprinc) to the groups that have rights granted to them ($groupprinc)
-    $self->Join( ALIAS1 => $cgm, FIELD1 => 'MemberId',
-                 ALIAS2 => $userprinc, FIELD2 => 'id' );
+    my $group_members = $self->_JoinGroupMembers( %args );

 
     foreach my $groupid (@{$args{'Groups'}}) {
 
     foreach my $groupid (@{$args{'Groups'}}) {

-        $self->Limit(ALIAS => $cgm, FIELD => 'GroupId', VALUE => $groupid, QUOTEVALUE => 0, ENTRYAGGREGATOR=> 'OR')
-
+        $self->Limit( ALIAS           => $group_members,
+                      FIELD           => 'GroupId',
+                      VALUE           => $groupid,
+                      QUOTEVALUE      => 0,
+                      ENTRYAGGREGATOR => 'OR',
+                    );

     }
 }
 # }}}
     }
 }
 # }}}