fix cdr svcnum link
[freeside.git] / rt / lib / RT / Principal_Overlay.pm
index 1986470..a8e8f3c 100644 (file)
@@ -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,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.
 # 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 }}}
+
 #
 
 package RT::Principal;
 #
 
 package RT::Principal;
@@ -50,12 +53,10 @@ package RT::Principal;
 use strict;
 use warnings;
 
 use strict;
 use warnings;
 
-no warnings qw(redefine);
-
 use Cache::Simple::TimedExpiry;
 
 
 use Cache::Simple::TimedExpiry;
 
 
-
+use RT;
 use RT::Group;
 use RT::User;
 
 use RT::Group;
 use RT::User;
 
@@ -74,12 +75,11 @@ Returns undef, otherwise
 
 sub IsGroup {
     my $self = shift;
 
 sub IsGroup {
     my $self = shift;
-    if ($self->PrincipalType eq 'Group') {
-        return(1);
-    }
-    else {
-        return undef;
+    if ( defined $self->PrincipalType && 
+            $self->PrincipalType eq 'Group' ) {
+        return 1;
     }
     }
+    return undef;
 }
 
 # }}}
 }
 
 # }}}
@@ -116,18 +116,18 @@ Returns the user or group associated with this principal
 sub Object {
     my $self = shift;
 
 sub Object {
     my $self = shift;
 
-    unless ($self->{'object'}) {
-    if ($self->IsUser) {
-       $self->{'object'} = RT::User->new($self->CurrentUser);
-    }
-    elsif ($self->IsGroup) {
-        $self->{'object'}  = RT::Group->new($self->CurrentUser);
-    }
-    else { 
-        $RT::Logger->crit("Found a principal (".$self->Id.") that was neither a user nor a group");
-        return(undef);
-    }
-    $self->{'object'}->Load($self->ObjectId());
+    unless ( $self->{'object'} ) {
+        if ( $self->IsUser ) {
+           $self->{'object'} = RT::User->new($self->CurrentUser);
+        }
+        elsif ( $self->IsGroup ) {
+            $self->{'object'}  = RT::Group->new($self->CurrentUser);
+        }
+        else { 
+            $RT::Logger->crit("Found a principal (".$self->Id.") that was neither a user nor a group");
+            return(undef);
+        }
+        $self->{'object'}->Load( $self->ObjectId() );
     }
     return ($self->{'object'});
 
     }
     return ($self->{'object'});
 
@@ -152,29 +152,27 @@ A helper function which calls RT::ACE->Create
 
 sub GrantRight {
     my $self = shift;
 
 sub GrantRight {
     my $self = shift;
-    my %args = ( Right => undef,
-                Object => undef,
-                @_);
-
-
-    unless ($args{'Right'}) {
-        return(0, $self->loc("Invalid Right"));
-    }
-
+    my %args = (
+        Right => undef,
+        Object => undef,
+        @_
+    );
 
     #ACL check handled in ACE.pm
     my $ace = RT::ACE->new( $self->CurrentUser );
 
 
     #ACL check handled in ACE.pm
     my $ace = RT::ACE->new( $self->CurrentUser );
 
-
     my $type = $self->_GetPrincipalTypeForACL();
 
     my $type = $self->_GetPrincipalTypeForACL();
 
+    RT->System->QueueCacheNeedsUpdate(1) if $args{'Right'} eq 'SeeQueue';
+
     # If it's a user, we really want to grant the right to their 
     # user equivalence group
     # If it's a user, we really want to grant the right to their 
     # user equivalence group
-        return ( $ace->Create(RightName => $args{'Right'},
-                          Object => $args{'Object'},
-                          PrincipalType =>  $type,
-                          PrincipalId => $self->Id
-                          ) );
+    return $ace->Create(
+        RightName     => $args{'Right'},
+        Object        => $args{'Object'},
+        PrincipalType => $type,
+        PrincipalId   => $self->Id,
+    );
 }
 # }}}
 
 }
 # }}}
 
@@ -195,7 +193,7 @@ sub RevokeRight {
 
     my $self = shift;
     my %args = (
 
     my $self = shift;
     my %args = (
-        Right      => undef,
+        Right  => undef,
         Object => undef,
         @_
     );
         Object => undef,
         @_
     );
@@ -208,24 +206,23 @@ sub RevokeRight {
     my $type = $self->_GetPrincipalTypeForACL();
 
     my $ace = RT::ACE->new( $self->CurrentUser );
     my $type = $self->_GetPrincipalTypeForACL();
 
     my $ace = RT::ACE->new( $self->CurrentUser );
-    $ace->LoadByValues(
+    my ($status, $msg) = $ace->LoadByValues(
         RightName     => $args{'Right'},
         RightName     => $args{'Right'},
-        Object    => $args{'Object'},
+        Object        => $args{'Object'},
         PrincipalType => $type,
         PrincipalId   => $self->Id
     );
 
         PrincipalType => $type,
         PrincipalId   => $self->Id
     );
 
-    unless ( $ace->Id ) {
-        return ( 0, $self->loc("ACE not found") );
-    }
-    return ( $ace->Delete );
+    RT->System->QueueCacheNeedsUpdate(1) if $args{'Right'} eq 'SeeQueue';
+    return ($status, $msg) unless $status;
+    return $ace->Delete;
 }
 
 # }}}
 
 }
 
 # }}}
 
-# {{{ sub _CleanupInvalidDelegations
+# {{{ sub CleanupInvalidDelegations
 
 
-=head2 sub _CleanupInvalidDelegations { InsideTransaction => undef }
+=head2 sub CleanupInvalidDelegations { InsideTransaction => undef }
 
 Revokes all ACE entries delegated by this principal which are
 inconsistent with this principal's current delegation rights.  Does
 
 Revokes all ACE entries delegated by this principal which are
 inconsistent with this principal's current delegation rights.  Does
@@ -247,15 +244,19 @@ and logs an internal error if the deletion fails (should not happen).
 # This is currently just a stub for the methods of the same name in
 # RT::User and RT::Group.
 
 # This is currently just a stub for the methods of the same name in
 # RT::User and RT::Group.
 
-sub _CleanupInvalidDelegations {
+# backcompat for 3.8.8 and before
+*_CleanupInvalidDelegations = \&CleanupInvalidDelegations;
+
+sub CleanupInvalidDelegations {
     my $self = shift;
     unless ( $self->Id ) {
        $RT::Logger->warning("Principal not loaded.");
        return (undef);
     }
     my $self = shift;
     unless ( $self->Id ) {
        $RT::Logger->warning("Principal not loaded.");
        return (undef);
     }
-    return ($self->Object->_CleanupInvalidDelegations(@_));
+    return ($self->Object->CleanupInvalidDelegations(@_));
 }
 
 }
 
+
 # }}}
 
 # {{{ sub HasRight
 # }}}
 
 # {{{ sub HasRight
@@ -301,10 +302,18 @@ sub HasRight {
         return (undef);
     }
 
         return (undef);
     }
 
-    $args{EquivObjects} = [ @{ $args{EquivObjects} } ] if $args{EquivObjects};
+    my $canonic_name = RT::ACE->CanonicalizeRightName( $args{'Right'} );
+    unless ( $canonic_name ) {
+        $RT::Logger->error("Invalid right. Couldn't canonicalize right '$args{'Right'}'");
+        return undef;
+    }
+    $args{'Right'} = $canonic_name;
+
+    $args{'EquivObjects'} = [ @{ $args{'EquivObjects'} } ]
+        if $args{'EquivObjects'};
 
     if ( $self->Disabled ) {
 
     if ( $self->Disabled ) {
-        $RT::Logger->error( "Disabled User:  "
+        $RT::Logger->debug( "Disabled User #"
               . $self->id
               . " failed access check for "
               . $args{'Right'} );
               . $self->id
               . " failed access check for "
               . $args{'Right'} );
@@ -315,60 +324,49 @@ sub HasRight {
         && UNIVERSAL::can( $args{'Object'}, 'id' )
         && $args{'Object'}->id ) {
 
         && UNIVERSAL::can( $args{'Object'}, 'id' )
         && $args{'Object'}->id ) {
 
-        push( @{ $args{'EquivObjects'} }, $args{Object} );
+        push @{ $args{'EquivObjects'} }, $args{'Object'};
     }
     else {
         $RT::Logger->crit("HasRight called with no valid object");
         return (undef);
     }
 
     }
     else {
         $RT::Logger->crit("HasRight called with no valid object");
         return (undef);
     }
 
-    # If this object is a ticket, we care about ticket roles and queue roles
-    if ( UNIVERSAL::isa( $args{'Object'} => 'RT::Ticket' ) ) {
 
 
-        # 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.
-        push( @{ $args{'EquivObjects'} }, $args{'Object'}->QueueObj );
+    unshift @{ $args{'EquivObjects'} }, $args{'Object'}->ACLEquivalenceObjects;
 
 
-    }
+    unshift @{ $args{'EquivObjects'} }, $RT::System
+        unless $self->can('_IsOverrideGlobalACL')
+               && $self->_IsOverrideGlobalACL( $args{'Object'} );
 
     # {{{ If we've cached a win or loss for this lookup say so
 
 
     # {{{ If we've cached a win or loss for this lookup say so
 
-    # {{{ Construct a hashkey to cache decisions in
-    my $hashkey = do {
-        no warnings 'uninitialized';
-
-        # We don't worry about the hash ordering, as this is only
-        # temporarily used; also if the key changes it would be
-        # invalidated anyway.
-        join(
-            ";:;",
-            $self->Id,
-            map {
-                $_,    # the key of each arguments
-                  ( $_ eq 'EquivObjects' )    # for object arrayref...
-                  ? map( _ReferenceId($_), @{ $args{$_} } )    # calculate each
-                  : _ReferenceId( $args{$_} )    # otherwise just the value
-              } keys %args
-        );
-    };
-
-    # }}}
-
-    # Returns undef on cache miss
-    my $cached_answer = $_ACL_CACHE->fetch($hashkey);
-    if ( defined $cached_answer ) {
-        if ( $cached_answer == 1 ) {
-            return (1);
-        }
-        elsif ( $cached_answer == -1 ) {
-            return (undef);
-        }
+    # Construct a hashkeys to cache decisions:
+    # 1) full_hashkey - key for any result and for full combination of uid, right and objects
+    # 2) short_hashkey - one key for each object to store positive results only, it applies
+    # only to direct group rights and partly to role rights
+    my $self_id = $self->id;
+    my $full_hashkey = join ";:;", $self_id, $args{'Right'};
+    foreach ( @{ $args{'EquivObjects'} } ) {
+        my $ref_id = _ReferenceId($_);
+        $full_hashkey .= ";:;$ref_id";
+
+        my $short_hashkey = join ";:;", $self_id, $args{'Right'}, $ref_id;
+        my $cached_answer = $_ACL_CACHE->fetch($short_hashkey);
+        return $cached_answer > 0 if defined $cached_answer;
+    }
+
+    {
+        my $cached_answer = $_ACL_CACHE->fetch($full_hashkey);
+        return $cached_answer > 0 if defined $cached_answer;
     }
 
     }
 
-    my $hitcount = $self->_HasRight( %args );
 
 
-    $_ACL_CACHE->set( $hashkey => $hitcount? 1:-1 );
+    my ($hitcount, $via_obj) = $self->_HasRight( %args );
+
+    $_ACL_CACHE->set( $full_hashkey => $hitcount? 1: -1 );
+    $_ACL_CACHE->set( "$self_id;:;$args{'Right'};:;$via_obj" => 1 )
+        if $via_obj && $hitcount;
+
     return ($hitcount);
 }
 
     return ($hitcount);
 }
 
@@ -381,55 +379,95 @@ Low level HasRight implementation, use HasRight method instead.
 sub _HasRight
 {
     my $self = shift;
 sub _HasRight
 {
     my $self = shift;
+    {
+        my ($hit, @other) = $self->_HasGroupRight( @_ );
+        return ($hit, @other) if $hit;
+    }
+    {
+        my ($hit, @other) = $self->_HasRoleRight( @_ );
+        return ($hit, @other) if $hit;
+    }
+    return (0);
+}
+
+# this method handles role rights partly in situations
+# where user plays role X on an object and as well the right is
+# assigned to this role X of the object, for example right CommentOnTicket
+# is granted to Cc role of a queue and user is in cc list of the queue
+sub _HasGroupRight
+{
+    my $self = shift;
     my %args = (
         Right        => undef,
     my %args = (
         Right        => undef,
-        Object       => undef,
         EquivObjects => [],
         @_
     );
         EquivObjects => [],
         @_
     );
-
     my $right = $args{'Right'};
     my $right = $args{'Right'};
-    my @objects = @{ $args{'EquivObjects'} };
-
-    # If an object is defined, we want to look at rights for that object
-
-    push( @objects, 'RT::System' )
-      unless $self->can('_IsOverrideGlobalACL')
-             && $self->_IsOverrideGlobalACL( $args{Object} );
-
-    my ($check_roles, $check_objects) = ('','');
-    if( @objects ) {
-        my @role_clauses;
-        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 $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 @role_clauses, "($role_clause)";
-
-            my $object_clause = "ACL.ObjectType = '$type'";
-            $object_clause   .= " AND ACL.ObjectId = $id" if $id;
-            push @object_clauses, "($object_clause)";
-        }
-
-        $check_roles .= join ' OR ', @role_clauses;
-        $check_objects = join ' OR ', @object_clauses;
-    }
 
 
-    my $query_base =
-      "SELECT ACL.id from ACL, Groups, Principals, CachedGroupMembers WHERE  " .
+    my $query =
+      "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId " .
+      "FROM ACL, Principals, CachedGroupMembers WHERE " .
 
       # Only find superuser or rights with the name $right
 
       # Only find superuser or rights with the name $right
-      "(ACL.RightName = 'SuperUser' OR  ACL.RightName = '$right') "
+      "(ACL.RightName = 'SuperUser' OR ACL.RightName = '$right') "
 
       # Never find disabled groups.
 
       # Never find disabled groups.
+      . "AND Principals.id = ACL.PrincipalId "
+      . "AND Principals.PrincipalType = 'Group' "
       . "AND Principals.Disabled = 0 "
       . "AND Principals.Disabled = 0 "
+
+      # See if the principal is a member of the group recursively or _is the rightholder_
+      # never find recursively disabled group members
+      # also, check to see if the right is being granted _directly_ to this principal,
+      #  as is the case when we want to look up group rights
+      . "AND CachedGroupMembers.GroupId  = ACL.PrincipalId "
+      . "AND CachedGroupMembers.GroupId  = Principals.id "
+      . "AND CachedGroupMembers.MemberId = ". $self->Id ." "
+      . "AND CachedGroupMembers.Disabled = 0 ";
+
+    my @clauses;
+    foreach my $obj ( @{ $args{'EquivObjects'} } ) {
+        my $type = ref( $obj ) || $obj;
+        my $clause = "ACL.ObjectType = '$type'";
+
+        if ( ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id ) {
+            $clause .= " AND ACL.ObjectId = ". $obj->id;
+        }
+
+        push @clauses, "($clause)";
+    }
+    if ( @clauses ) {
+        $query .= " AND (". join( ' OR ', @clauses ) .")";
+    }
+
+    $self->_Handle->ApplyLimits( \$query, 1 );
+    my ($hit, $obj, $id) = $self->_Handle->FetchResult( $query );
+    return (0) unless $hit;
+
+    $obj .= "-$id" if $id;
+    return (1, $obj);
+}
+
+sub _HasRoleRight
+{
+    my $self = shift;
+    my %args = (
+        Right        => undef,
+        EquivObjects => [],
+        @_
+    );
+
+    my @roles = $self->RolesWithRight( %args );
+    return 0 unless @roles;
+
+    my $right = $args{'Right'};
+
+    my $query =
+      "SELECT Groups.id "
+      . "FROM Groups, Principals, CachedGroupMembers WHERE "
+
+      # Never find disabled things
+      . "Principals.Disabled = 0 "
       . "AND CachedGroupMembers.Disabled = 0 "
 
       # We always grant rights to Groups
       . "AND CachedGroupMembers.Disabled = 0 "
 
       # We always grant rights to Groups
@@ -443,31 +481,94 @@ sub _HasRight
       . "AND Principals.id = CachedGroupMembers.GroupId "
       . "AND CachedGroupMembers.MemberId = ". $self->Id ." "
 
       . "AND Principals.id = CachedGroupMembers.GroupId "
       . "AND CachedGroupMembers.MemberId = ". $self->Id ." "
 
-      # Make sure the rights apply to the entire system or to the object in question
-      . "AND ($check_objects) ";
+      . "AND (". join(' OR ', map "Groups.Type = '$_'", @roles ) .")"
+    ;
+
+    my (@object_clauses);
+    foreach my $obj ( @{ $args{'EquivObjects'} } ) {
+        my $type = ref($obj)? ref($obj): $obj;
+        my $id;
+        $id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
+
+        my $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.
+        $clause .= " AND Groups.Instance = '$id'" if $id;
+        push @object_clauses, "($clause)";
+    }
+    $query .= " AND (". join( ' OR ', @object_clauses ) .")";
 
 
-    # The groups query does the query based on group membership and individual user rights
-    my $groups_query = $query_base
-      # limit the result set to groups of types ACLEquivalence (user),
-      # UserDefined, SystemInternal and Personal. All this we do
-      # via (ACL.PrincipalType = 'Group') condition
-      . "AND ACL.PrincipalId = Principals.id "
-      . "AND ACL.PrincipalType = 'Group' ";
+    $self->_Handle->ApplyLimits( \$query, 1 );
+    my ($hit) = $self->_Handle->FetchResult( $query );
+    return (1) if $hit;
 
 
-    $self->_Handle->ApplyLimits( \$groups_query, 1 ); #only return one result
-    my $hitcount = $self->_Handle->FetchResult($groups_query);
-    return 1 if $hitcount; # get out of here if success
+    return 0;
+}
 
 
-    # The roles query does the query based on roles
-    my $roles_query = $query_base
-      . "AND ACL.PrincipalType = Groups.Type "
-      . "AND ($check_roles) ";
-    $self->_Handle->ApplyLimits( \$roles_query, 1 ); #only return one result
+=head2 RolesWithRight
 
 
-    $hitcount = $self->_Handle->FetchResult($roles_query);
-    return 1 if $hitcount; # get out of here if success
+Returns list with names of roles that have right on
+set of objects. Takes Right, EquiveObjects,
+IncludeSystemRights and IncludeSuperusers arguments.
 
 
-    return 0;
+IncludeSystemRights is true by default, rights
+granted on system level are not accouned when option
+is set to false value.
+
+IncludeSuperusers is true by default, SuperUser right
+is not checked if it's set to false value.
+
+=cut
+
+sub RolesWithRight {
+    my $self = shift;
+    my %args = (
+        Right               => undef,
+        IncludeSystemRights => 1,
+        IncludeSuperusers   => 1,
+        EquivObjects        => [],
+        @_
+    );
+    my $right = $args{'Right'};
+
+    my $query =
+        "SELECT DISTINCT PrincipalType FROM ACL"
+        # Only find superuser or rights with the name $right
+        ." WHERE ( RightName = '$right' "
+        # Check SuperUser if we were asked to
+        . ($args{'IncludeSuperusers'}? "OR RightName = 'SuperUser' " : '' )
+        .")"
+        # we need only roles
+        ." AND PrincipalType != 'Group'"
+    ;
+
+    # skip rights granted on system level if we were asked to
+    unless ( $args{'IncludeSystemRights'} ) {
+        $query .= " AND ObjectType != 'RT::System'";
+    }
+
+    my (@object_clauses);
+    foreach my $obj ( @{ $args{'EquivObjects'} } ) {
+        my $type = ref($obj)? ref($obj): $obj;
+        my $id;
+        $id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
+
+        my $object_clause = "ObjectType = '$type'";
+        $object_clause   .= " AND ObjectId = $id" if $id;
+        push @object_clauses, "($object_clause)";
+    }
+    # find ACLs that are related to our objects only
+    $query .= " AND (". join( ' OR ', @object_clauses ) .")"
+        if @object_clauses;
+
+    my $dbh = $RT::Handle->dbh;
+    my $roles = $dbh->selectcol_arrayref($query);
+    unless ( $roles ) {
+        $RT::Logger->warning( $dbh->errstr );
+        return ();
+    }
+    return @$roles;
 }
 
 # }}}
 }
 
 # }}}
@@ -487,8 +588,9 @@ Cleans out and reinitializes the user rights cache
 
 sub InvalidateACLCache {
     $_ACL_CACHE = Cache::Simple::TimedExpiry->new();
 
 sub InvalidateACLCache {
     $_ACL_CACHE = Cache::Simple::TimedExpiry->new();
-    $_ACL_CACHE->expire_after($RT::ACLCacheLifetime||60);
-
+    my $lifetime;
+    $lifetime = $RT::Config->Get('ACLCacheLifetime') if $RT::Config;
+    $_ACL_CACHE->expire_after( $lifetime || 60 );
 }
 
 # }}}
 }
 
 # }}}