Merge branch 'security/4.2/acl-addrecord' into security/4.2.10-relengrt-4.2.10

12 files changed, 81 insertions, 177 deletions

diff --git a/lib/RT/ACL.pm b/lib/RT/ACL.pm
index 4c90f1839e..89ca69f2d6 100644
--- a/lib/RT/ACL.pm
+++ b/lib/RT/ACL.pm
@@ -188,34 +188,21 @@ sub LimitToPrincipal {
 
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $ACE = $self->SUPER::Next();
     # Short-circuit having to load up the ->Object
-    return $ACE
-        if $self->CurrentUser->PrincipalObj->Id == RT->SystemUser->Id;
-    if ( ( defined($ACE) ) and ( ref($ACE) ) ) {
-
-        if ( $self->CurrentUser->HasRight( Right  => 'ShowACL',
-                                           Object => $ACE->Object )
-             or $self->CurrentUser->HasRight( Right  => 'ModifyACL',
-                                              Object => $ACE->Object )
-          ) {
-            return ($ACE);
-        }
+    return $self->SUPER::AddRecord( $record )
+        if $record->CurrentUser->PrincipalObj->Id == RT->SystemUser->Id;
 
-        #If the user doesn't have the right to show this ACE
-        else {
-            return ( $self->Next() );
-        }
-    }
-
-    #if there never was any ACE
-    else {
-        return (undef);
-    }
+    my $obj = $record->Object;
+    return unless $self->CurrentUser->HasRight( Right  => 'ShowACL',
+                                                Object => $obj )
+               or $self->CurrentUser->HasRight( Right  => 'ModifyACL',
+                                                Object => $obj );
 
+    return $self->SUPER::AddRecord( $record );
 }
 
 # The singular of ACL is ACE.
diff --git a/lib/RT/Articles.pm b/lib/RT/Articles.pm
index f604acc7d5..4b85ebab64 100644
--- a/lib/RT/Articles.pm
+++ b/lib/RT/Articles.pm
@@ -64,33 +64,19 @@ sub _Init {
     return $self->SUPER::_Init( @_ );
 }
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next article that this user can see.
+Overrides the collection to ensure that only Articles the user can see
+are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Object = $self->SUPER::Next();
-    if ( ( defined($Object) ) and ( ref($Object) ) ) {
-
-        if ( $Object->CurrentUserHasRight('ShowArticle') ) {
-            return ($Object);
-        }
-
-        #If the user doesn't have the right to show this Object
-        else {
-            return ( $self->Next() );
-        }
-    }
-
-    #if there never was any queue
-    else {
-        return (undef);
-    }
-
+    return unless $record->CurrentUserHasRight('ShowArticle');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 Limit { FIELD  => undef, OPERATOR => '=', VALUE => 'undef'} 
diff --git a/lib/RT/Attachments.pm b/lib/RT/Attachments.pm
index 0a3bad180b..13cf5cf84e 100644
--- a/lib/RT/Attachments.pm
+++ b/lib/RT/Attachments.pm
@@ -215,19 +215,12 @@ sub LimitByTicket {
     return;
 }
 
-# {{{ sub Next
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Attachment = $self->SUPER::Next;
-    return $Attachment unless $Attachment;
-
-    if ( $Attachment->TransactionObj->CurrentUserCanSee ) {
-        return $Attachment;
-    } else {
-        # If the user doesn't have the right to show this ticket
-        return $self->Next;
-    }
+    return unless $record->TransactionObj->CurrentUserCanSee;
+    return $self->SUPER::AddRecord( $record );
 }
 
 RT::Base->_ImportOverlays();
diff --git a/lib/RT/Classes.pm b/lib/RT/Classes.pm
index d9e61ff74e..bf55d523d2 100644
--- a/lib/RT/Classes.pm
+++ b/lib/RT/Classes.pm
@@ -64,32 +64,19 @@ sub Table {'Classes'}
     return ($self->SUPER::_Init(@_));
  }
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next Object that this user can see.
+Overrides the collection to ensure that only Classes the user can
+see are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $Object = $self->SUPER::Next();
-    if ((defined($Object)) and (ref($Object))) {
-   if ( $Object->CurrentUserHasRight('SeeClass') ) {
-        return($Object);
-    }
-
-    #If the user doesn't have the right to show this Object
-    else {
-        return($self->Next());
-    }
-    }
-    #if there never was any Object
-    else {
-    return(undef);
-    }
-
+    return unless $record->CurrentUserHasRight('SeeClass');
+    return $self->SUPER::AddRecord( $record );
 }
 
 sub _SingularClass { "RT::Class" }
diff --git a/lib/RT/CustomFields.pm b/lib/RT/CustomFields.pm
index f4ccd44db5..a93bfc80ce 100644
--- a/lib/RT/CustomFields.pm
+++ b/lib/RT/CustomFields.pm
@@ -378,22 +378,20 @@ sub _OCFAlias {
 }
 
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next custom field that this user can see.
+Overrides the collection to ensure that only custom fields the user can
+see are returned; also propagates down the L</ContextObject>.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $CF = $self->SUPER::Next();
-    return $CF unless $CF;
-
-    $CF->SetContextObject( $self->ContextObject );
-
-    return $self->Next unless $CF->CurrentUserHasRight('SeeCustomField');
-    return $CF;
+    $record->SetContextObject( $self->ContextObject );
+    return unless $record->CurrentUserHasRight('SeeCustomField');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 NewItem
diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 7e804619cc..6d9eeb6bae 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -255,8 +255,7 @@ sub _PrivacyObjects {
 
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
-    $groups->WithMember( PrincipalId => $CurrentUser->Id,
-                         Recursively => 1 );
+    $groups->WithCurrentUser;
     push @objects, @{ $groups->ItemsArrayRef };
 
     push @objects, RT::System->new($CurrentUser);
@@ -386,10 +385,7 @@ sub ObjectsForLoading {
         Right             => 'SeeGroupDashboard',
         IncludeSuperusers => $args{IncludeSuperuserGroups},
     );
-    $groups->WithMember(
-        Recursively => 1,
-        PrincipalId => $CurrentUser->UserObj->PrincipalId
-    );
+    $groups->WithCurrentUser;
     my $attrs = $groups->Join(
         ALIAS1 => 'main',
         FIELD1 => 'id',
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index cd0aa9ffcd..4d0652851b 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -272,6 +272,15 @@ sub WithMember {
     return $members;
 }
 
+sub WithCurrentUser {
+    my $self = shift;
+    $self->{with_current_user} = 1;
+    return $self->WithMember(
+        PrincipalId => $self->CurrentUser->PrincipalId,
+        Recursively => 1,
+    );
+}
+
 sub WithoutMember {
     my $self = shift;
     my %args = (
@@ -459,22 +468,16 @@ sub LimitToDeleted {
 
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    # Don't show groups which the user isn't allowed to see.
+    # If we've explicitly limited to groups the user is a member of (for
+    # dashboard or savedsearch privacy objects), skip the ACL.
+    return unless $self->{with_current_user}
+        or $record->CurrentUserHasRight('SeeGroup');
 
-    my $Group = $self->SUPER::Next();
-    if ((defined($Group)) and (ref($Group))) {
-        unless ($Group->CurrentUserHasRight('SeeGroup')) {
-            return $self->Next();
-        }
-
-        return $Group;
-    }
-    else {
-        return undef;
-    }
+    return $self->SUPER::AddRecord( $record );
 }
 
 
diff --git a/lib/RT/SavedSearch.pm b/lib/RT/SavedSearch.pm
index 4fe6323a31..4dd869bf24 100644
--- a/lib/RT/SavedSearch.pm
+++ b/lib/RT/SavedSearch.pm
@@ -162,8 +162,7 @@ sub _PrivacyObjects {
 
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
-    $groups->WithMember( PrincipalId => $CurrentUser->Id,
-                         Recursively => 1 );
+    $groups->WithCurrentUser;
     if ($has_attr) {
         my $attrs = $groups->Join(
             ALIAS1 => 'main',
diff --git a/lib/RT/Scrips.pm b/lib/RT/Scrips.pm
index ef8213c391..85f1961f3a 100644
--- a/lib/RT/Scrips.pm
+++ b/lib/RT/Scrips.pm
@@ -238,35 +238,19 @@ sub ApplySortOrder {
     } );
 }
 
-# {{{ sub Next 
+=head2 AddRecord
 
-=head2 Next
-
-Returns the next scrip that this user can see.
+Overrides the collection to ensure that only scrips the user can see are
+returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $Scrip = $self->SUPER::Next();
-    if ((defined($Scrip)) and (ref($Scrip))) {
-
-        if ($Scrip->CurrentUserHasRight('ShowScrips')) {
-            return($Scrip);
-        }
-
-        #If the user doesn't have the right to show this scrip
-        else {
-            return($self->Next());
-        }
-    }
-    #if there never was any scrip
-    else {
-        return(undef);
-    }
-
+    return unless $record->CurrentUserHasRight('ShowScrips');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 Apply
diff --git a/lib/RT/Template.pm b/lib/RT/Template.pm
index 23f150b255..d299e0b5b2 100644
--- a/lib/RT/Template.pm
+++ b/lib/RT/Template.pm
@@ -842,10 +842,14 @@ sub CompileCheck {
 sub CurrentUserCanRead {
     my $self =shift;
 
-    return 1 if $self->CurrentUserHasQueueRight('ShowTemplate');
-
-    return $self->CurrentUser->HasRight( Right =>'ShowGlobalTemplates', Object => $RT::System )
-        if !$self->QueueObj->Id;
+    if ($self->__Value('Queue')) {
+        my $queue = RT::Queue->new( RT->SystemUser );
+        $queue->Load( $self->__Value('Queue'));
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowTemplate', Object => $queue );
+    } else {
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowGlobalTemplates', Object => $RT::System );
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowTemplate',        Object => $RT::System );
+    }
 
     return;
 }
diff --git a/lib/RT/Templates.pm b/lib/RT/Templates.pm
index ef381611df..93ed4fc80f 100644
--- a/lib/RT/Templates.pm
+++ b/lib/RT/Templates.pm
@@ -125,37 +125,19 @@ sub LimitToQueue {
 }
 
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next template that this user can see.
+Overrides the collection to ensure that only templates the user can see
+are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $templ = $self->SUPER::Next();
-    if ((defined($templ)) and (ref($templ))) {
-
-        # If it's part of a queue, and the user can read templates in
-        # that queue, or the user can globally read templates, show it
-        if ($templ->Queue && $templ->CurrentUserHasQueueRight('ShowTemplate') or
-            $templ->CurrentUser->HasRight(Object => $RT::System, Right => 'ShowTemplate') or
-            $templ->CurrentUser->HasRight(Object => $RT::System, Right => 'ShowGlobalTemplates')) {
-            return($templ);
-        }
-
-        #If the user doesn't have the right to show this template
-        else {
-            return($self->Next());
-        }
-    }
-    #if there never was any template
-    else {
-        return(undef);
-    }
-
+    return unless $record->CurrentUserCanRead;
+    return $self->SUPER::AddRecord( $record );
 }
 
 RT::Base->_ImportOverlays();
diff --git a/lib/RT/Transactions.pm b/lib/RT/Transactions.pm
index 4d00e2f524..6794e527fa 100644
--- a/lib/RT/Transactions.pm
+++ b/lib/RT/Transactions.pm
@@ -130,27 +130,12 @@ sub LimitToTicket {
 }
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Transaction = $self->SUPER::Next();
-    if ((defined($Transaction)) and (ref($Transaction))) {
-        # If the user can see the transaction's type, then they can
-        #  see the transaction and we should hand it back.
-        if ($Transaction->Type) {
-            return($Transaction);
-        }
-
-        #If the user doesn't have the right to show this ticket
-        else {
-            return($self->Next());
-        }
-    }
-
-    #if there never was any ticket
-    else {
-        return(undef);
-    }
+    return unless $record->CurrentUserCanSee;
+    return $self->SUPER::AddRecord($record);
 }
 
 RT::Base->_ImportOverlays();