Hi all

I've noticed that the WhoBelongToGroups method of user search class
RT::Users unconditionally calls LimitToPrivileged, making it useless for
when you want to find both privileged and unprivileged users, or when
you want to find only unprivileged users where you've already called
LimitToUnprivileged . This behaviour isn't documented and there's a comment:

    # Unprivileged users can't be granted real system rights.
    # is this really the right thing to be saying?
    $self->LimitToPrivileged();

that suggests uncertainty about whether it's correct.

I'm running 4.07 but this code remains in 4.10 and in 4.2 git master HEAD.

The attached patch documents the current behaviour and adds a flag to
override it per-call. It applies to 4.0 and master HEAD. A pull request
has been sent with the changes:

https://github.com/bestpractical/rt/pull/47

Filed as RT ticket 22989.

Anyone who wants to apply this change in the mean time can do so by
grabbing lib/RT/Users.pm, extracting the WhoBelongToGroups method from
it, and adding it to $localrtlib/RT/Users_Local.pm like this:

-----
use strict;
no warnings qw(redefine);
package RT::Users;

# paste WhoBelongToGroups method here

1;
----


-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



>From 6dc19ce480ed5449e4867e07c44562e30e661e8f Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Thu, 7 Mar 2013 12:54:26 +0800
Subject: [PATCH] Allow RT::Users->WhoBelongToGroups to optionally return
 unprivileged users

The WhoBelongToGroups filter should really not apply LimitToPrivileged at
all, but we can't remove it without breaking security assumptions elsewhere
in RT or in 3rd party code. So a new option IncludeUnprivileged is added, a
boolean that defaults to false. It can be set to true to disable the
LimitToPrivileged filter.

The documentation of the method is amended to explain that it filters out
unprivileged members by default.
---
 lib/RT/Users.pm | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 85a9b7b..a88f320 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -542,21 +542,31 @@ sub WhoHaveGroupRight
 }
 
 
-=head2 WhoBelongToGroups { Groups => ARRAYREF, IncludeSubgroupMembers => 1 }
+=head2 WhoBelongToGroups { Groups => ARRAYREF, IncludeSubgroupMembers => 1, IncludeUnprivileged => 0 }
+
+Return members who belong to any of the groups passed in the groups whose IDs
+are included in the Groups arrayref.
+
+If IncludeSubgroupMembers is true (default) then members of any group that's a
+member of one of the passed groups are returned. If it's cleared then only
+direct member users are returned.
+
+If IncludeUnprivileged is false (default) then only privileged members are
+returned; otherwise either privileged or unprivileged group members may be
+returned.
 
 =cut
 
-# XXX: should be generalized
 sub WhoBelongToGroups {
     my $self = shift;
     my %args = ( Groups                 => undef,
                  IncludeSubgroupMembers => 1,
+                 IncludeUnprivileged    => 0,
                  @_ );
 
-    # Unprivileged users can't be granted real system rights.
-    # is this really the right thing to be saying?
-    $self->LimitToPrivileged();
-
+    if (!$args{'IncludeUnprivileged'}) {
+        $self->LimitToPrivileged();
+    }
     my $group_members = $self->_JoinGroupMembers( %args );
 
     foreach my $groupid (@{$args{'Groups'}}) {
-- 
1.8.1.2



-- 
RT training in Amsterdam, March 20-21: 
http://bestpractical.com/services/training.html

Help improve RT by taking our user survey: 
https://www.surveymonkey.com/s/N23JW9T

Reply via email to