[Rt-commit] rt branch, 4.2/see-queue-groups, created. rt-4.2.9-61-ga94cb39

[Alex Vandiver]

The branch, 4.2/see-queue-groups has been created
        at  a94cb3998c36019791d97264e039057967be94c3 (commit)

- Log -----------------------------------------------------------------
commit a94cb3998c36019791d97264e039057967be94c3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Dec 30 13:13:01 2014 -0500

    Always allow role and system groups to be enumerated
    
    Previously, the SeeGroup right controlled RT::Groups results even for
    internal groups; this caused the queue rights page for a user with
    AdminQueue but not SeeGroup to not list Everyone / Privileged /
    Unprivileged, nor the queue role groups.
    
    Allow system groups to always be seen, and role groups to be seen if the
    user can see the object the role group is on.  This is a broadening of
    the privileges that previously existed.
    
    Note that as this limit applies to ->Next, and not ->AddRecord, and as
    ->CurrentUserCanSee is not used to ACL ->_Value, this does not enforce a
    group ACL globally.  ->ItemsArrayRef will still return all matching
    groups, regardless of rights, and any explicitly loaded group can be
    examined.
    
    Moving the ACL to ->AddRecord is complicated at this time, as users need
    the ability to see groups which are watchers on tickets they can see,
    which is difficult to impose in a performant manner.
    
    Fixes: I#30416

diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index f38f7a1..ebe1cc1 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1358,15 +1358,28 @@ sub _Set {
 
 =head2 CurrentUserCanSee
 
-Always returns 1; unfortunately, for historical reasons, users have
-always been able to examine groups they have indirect access to, even if
-they do not have SeeGroup explicitly.
+Returns 1 if the group is user-defined and the user has SeeGroup on it;
+returns 1 for internal groups, or role groups on objects which the user
+has permissions to see.
 
 =cut
 
 sub CurrentUserCanSee {
     my $self = shift;
-    return 1;
+
+    if ($self->Domain eq "UserDefined") {
+        return $self->CurrentUserHasRight("SeeGroup");
+    } elsif ($self->Domain eq "SystemInternal") {
+        return 1;
+    } elsif ($self->Domain eq "ACLEquivalence") {
+        return 1;
+    } elsif ($self->RoleClass) {
+        my $role = $self->RoleGroupObject;
+        return $role->CurrentUserCanSee if $role->can("CurrentUserCanSee");
+        return 1;
+    } else {
+        return 1;
+    }
 }
 
 
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 1ecc312..820cc87 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -458,27 +458,18 @@ sub LimitToDeleted {
 }
 
 
-
 sub Next {
     my $self = shift;
 
-    # Don't show groups which the user isn't allowed to see.
+    my $group = $self->SUPER::Next();
+    return unless $group;
 
-    my $Group = $self->SUPER::Next();
-    if ((defined($Group)) and (ref($Group))) {
-        unless ($Group->CurrentUserHasRight('SeeGroup')) {
-            return $self->Next();
-        }
+    return $self->Next unless $group->CurrentUserCanSee;
 
-        return $Group;
-    }
-    else {
-        return undef;
-    }
+    return $group;
 }
 
 
-
 sub _DoSearch {
     my $self = shift;
 
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 6059eb0..6988766 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2854,7 +2854,7 @@ sub CurrentUserCanSee {
     my ($what, $txn) = @_;
     return 0 unless $self->CurrentUserHasRight('ShowTicket');
 
-    return 1 if $what ne "Transaction";
+    return 1 unless $what and $what eq "Transaction";
 
     # If it's a comment, we need to be extra special careful
     my $type = $txn->__Value('Type');

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