[Rt-commit] rt branch, 4.4/shared-setting-drop-group-member-limit, created. rt-4.4.2-76-g820bc10de

? sunnavy sunnavy at bestpractical.com
Tue Feb 13 14:43:16 EST 2018


The branch, 4.4/shared-setting-drop-group-member-limit has been created
        at  820bc10de9e5f00a0f8ff5c5c2a7d599b98780bc (commit)

- Log -----------------------------------------------------------------
commit a84748adf644ea8b19db02a092eb4330a273bae4
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Feb 14 02:24:11 2018 +0800

    remove group member limit for shared setting access(dashboards/savedsearches)
    
    Previously only members of a group can access the group's dashboards and
    savedsearches. This limit is inconvenient and also unnecessary
    considering we have corresponding rights to control the access limit.
    
    The new behavior drops member limit and totally depends on Rights. E.g.
    as long as a user has "ShowSavedSearches" right on the group, it's
    totally fine to allow the user to access the group's savedsearches.
    
    Global "ShowSavedSearches" means permission to access all the group
    savedsearches.

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 7f76853d9..95487c77c 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -247,6 +247,7 @@ sub PossibleHiddenSearches {
 
 sub _PrivacyObjects {
     my $self = shift;
+    RT->Deprecated( Instead => 'ObjectsForLoading', Remove => '4.6' );
 
     my @objects;
 
@@ -361,58 +362,57 @@ sub Subscription {
     return;
 }
 
-sub ObjectsForLoading {
+sub _ObjectsFor {
     my $self = shift;
-    my %args = (
-        IncludeSuperuserGroups => 1,
-        @_
-    );
-    my @objects;
+    my %args = @_;
 
-    # If you've been granted the SeeOwnDashboard global right (which you
-    # could have by way of global user right or global group right), you
-    # get to see your own dashboards
+    my @objects;
     my $CurrentUser = $self->CurrentUser;
+
+    # user level
     push @objects, $CurrentUser->UserObj
-        if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeOwnDashboard');
+        if $CurrentUser->HasRight(Object => $RT::System, Right => "$args{Type}OwnDashboard");
 
-    # Find groups for which: (a) you are a member of the group, and (b)
-    # you have been granted SeeGroupDashboard on (by any means), and (c)
-    # have at least one dashboard
-    my $groups = RT::Groups->new($CurrentUser);
+    # group level
+    my $groups = RT::Groups->new( $self->CurrentUser );
+    $groups->{for_shared_settings} = 1;
     $groups->LimitToUserDefinedGroups;
     $groups->ForWhichCurrentUserHasRight(
-        Right             => 'SeeGroupDashboard',
-        IncludeSuperusers => $args{IncludeSuperuserGroups},
-    );
-    $groups->WithCurrentUser;
-    my $attrs = $groups->Join(
-        ALIAS1 => 'main',
-        FIELD1 => 'id',
-        TABLE2 => 'Attributes',
-        FIELD2 => 'ObjectId',
-    );
-    $groups->Limit(
-        ALIAS => $attrs,
-        FIELD => 'ObjectType',
-        VALUE => 'RT::Group',
-    );
-    $groups->Limit(
-        ALIAS => $attrs,
-        FIELD => 'Name',
-        VALUE => 'Dashboard',
+        Right             => "$args{Type}GroupDashboard",
+        IncludeSuperusers => $args{IncludeSuperuserGroups} // 1,
     );
+    if ( $args{Type} ne 'Create' ) {
+        my $attrs = $groups->Join(
+            ALIAS1 => 'main',
+            FIELD1 => 'id',
+            TABLE2 => 'Attributes',
+            FIELD2 => 'ObjectId',
+        );
+        $groups->Limit(
+            ALIAS => $attrs,
+            FIELD => 'ObjectType',
+            VALUE => 'RT::Group',
+        );
+        $groups->Limit(
+            ALIAS => $attrs,
+            FIELD => 'Name',
+            VALUE => 'Dashboard',
+        );
+    }
     push @objects, @{ $groups->ItemsArrayRef };
 
-    # Finally, if you have been granted the SeeDashboard right (which
-    # you could have by way of global user right or global group right),
-    # you can see system dashboards.
+    # system level
     push @objects, RT::System->new($CurrentUser)
-        if $CurrentUser->HasRight(Object => $RT::System, Right => 'SeeDashboard');
+        if $CurrentUser->HasRight(Object => $RT::System, Right => "$args{Type}Dashboard");
 
     return @objects;
 }
 
+sub ObjectsForLoading   { shift->_ObjectsFor( Type => 'See', @_) }
+sub ObjectsForCreating  { shift->_ObjectsFor( Type => 'Create', @_ ) }
+sub ObjectsForModifying { shift->_ObjectsFor( Type => 'Modify', @_ ) }
+sub ObjectsForDeleting  { shift->_ObjectsFor( Type => 'Delete', @_ ) }
+
 sub CurrentUserCanCreateAny {
     my $self = shift;
     my @objects;
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 971a124db..38da78022 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -413,7 +413,7 @@ sub AddRecord {
 
     # 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}
+    return unless $self->{with_current_user} or $self->{for_shared_settings}
         or $record->CurrentUserHasRight('SeeGroup');
 
     return $self->SUPER::AddRecord( $record );
diff --git a/lib/RT/SavedSearch.pm b/lib/RT/SavedSearch.pm
index 8bb1452d2..e93cfe39b 100644
--- a/lib/RT/SavedSearch.pm
+++ b/lib/RT/SavedSearch.pm
@@ -158,6 +158,8 @@ sub Type {
 sub _PrivacyObjects {
     my $self        = shift;
     my ($has_attr) = @_;
+    RT->Deprecated( Instead => 'ObjectsForLoading', Remove => '4.6' );
+
     my $CurrentUser = $self->CurrentUser;
 
     my $groups = RT::Groups->new($CurrentUser);
@@ -185,11 +187,61 @@ sub _PrivacyObjects {
     return ( $CurrentUser->UserObj, @{ $groups->ItemsArrayRef() } );
 }
 
-sub ObjectsForLoading {
+sub _ObjectsFor {
     my $self = shift;
-    return grep { $self->CurrentUserCanSee($_) } $self->_PrivacyObjects( "SavedSearch" );
+    my %args = @_;
+
+    my @objects;
+    my $CurrentUser = $self->CurrentUser;
+
+    # user level
+    if ( $args{Type} eq 'Show' || $CurrentUser->HasRight(Object => $RT::System, Right => "ModifySelf") ) {
+        push @objects, $CurrentUser->UserObj;
+    }
+
+    # group level
+    my $groups = RT::Groups->new( $self->CurrentUser );
+    $groups->{for_shared_settings} = 1;
+    $groups->LimitToUserDefinedGroups;
+    $groups->ForWhichCurrentUserHasRight(
+        Right             => "$args{Type}SavedSearches",
+        IncludeSuperusers => $args{IncludeSuperuserGroups} // 1,
+    );
+    if ( $args{Type} eq 'Show' ) {
+        my $attrs = $groups->Join(
+            ALIAS1 => 'main',
+            FIELD1 => 'id',
+            TABLE2 => 'Attributes',
+            FIELD2 => 'ObjectId',
+        );
+        $groups->Limit(
+            ALIAS => $attrs,
+            FIELD => 'ObjectType',
+            VALUE => 'RT::Group',
+        );
+        $groups->Limit(
+            ALIAS => $attrs,
+            FIELD => 'Name',
+            VALUE => 'SavedSearch',
+        );
+    }
+    push @objects, @{ $groups->ItemsArrayRef };
+
+    # TODO system level is tricky.
+    # current behavior is normal users could add system saved searches to
+    # their 'RT at a glance' on MyRT.html but can't load them on search pages
+    # let's not add it and leave it to callers to decide for now
+
+    return @objects;
 }
 
+sub ObjectsForLoading   { shift->_ObjectsFor( Type => 'Show', @_ ) }
+
+# Creating/Modifying/Deleting share the same right of "EditSavedSearches"
+sub ObjectsForCreating  { shift->_ObjectsFor( Type => 'Edit', @_ ) }
+sub ObjectsForModifying { shift->_ObjectsFor( Type => 'Edit', @_ ) }
+sub ObjectsForDeleting  { shift->_ObjectsFor( Type => 'Edit', @_ ) }
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/SharedSetting.pm b/lib/RT/SharedSetting.pm
index 30f9e099c..8f8ee4e5c 100644
--- a/lib/RT/SharedSetting.pm
+++ b/lib/RT/SharedSetting.pm
@@ -110,8 +110,10 @@ sub Load {
             $self->{'Privacy'} = $privacy;
             $self->PostLoad();
 
-            return wantarray ? (0, $self->loc("Permission Denied")) : 0
-                unless $self->CurrentUserCanSee;
+            unless ( $self->CurrentUserCanSee ) {
+                undef $self->{$_} for qw/Attribute Id Privacy/;
+                return wantarray ? ( 0, $self->loc( "Permission Denied" ) ) : 0;
+            }
 
             my ($ok, $msg) = $self->PostLoadValidate;
             return wantarray ? ($ok, $msg) : $ok if !$ok;
@@ -414,22 +416,13 @@ sub _GetObject {
         return undef;
     }
 
-    # Do not allow the loading of a user object other than the current
-    # user, or of a group object of which the current user is not a member.
+    # Do not allow the loading of a user object other than the current user
 
     if ($obj_type eq 'RT::User' && $object->Id != $self->CurrentUser->UserObj->Id) {
         $RT::Logger->debug("Permission denied for user other than self");
         return undef;
     }
 
-    if (   $obj_type eq 'RT::Group'
-        && !$object->HasMemberRecursively($self->CurrentUser->PrincipalObj)
-        && !$self->CurrentUser->HasRight( Object => $RT::System, Right => 'SuperUser' ) ) {
-        $RT::Logger->debug("Permission denied, ".$self->CurrentUser->Name.
-                           " is not a member of group");
-        return undef;
-    }
-
     return $object;
 }
 

commit 8e0f5b6c566953545b0d6f406f953d2ae3a65dd8
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Feb 14 02:25:42 2018 +0800

    update pages for the shared setting access logic change(no group member limit)
    
    Superusers could shortcut rights check, so this change could cause
    superusers to load all the groups' dashboards on /Dashboard/index.html
    by default, which is inconvenient. To get around that, for web pages
    like that, even superusers need to have global "SeeGroupDashboard" to
    list all the groups' dashboards.

diff --git a/share/html/Dashboards/index.html b/share/html/Dashboards/index.html
index fb505668d..fe1d81102 100644
--- a/share/html/Dashboards/index.html
+++ b/share/html/Dashboards/index.html
@@ -50,4 +50,4 @@
 
 <& /Elements/ListActions &>
 
-<& /Dashboards/Elements/ShowDashboards &>
+<& /Dashboards/Elements/ShowDashboards, IncludeSuperuserGroups => 0 &>
diff --git a/share/html/Search/Elements/EditSearches b/share/html/Search/Elements/EditSearches
index 846a8448f..b589baae5 100644
--- a/share/html/Search/Elements/EditSearches
+++ b/share/html/Search/Elements/EditSearches
@@ -76,7 +76,7 @@
 <br />
 <hr />
 <span class="label"><&|/l&>Load saved search</&>:</span>
-<& SelectSearchesForObjects, Name => 'SavedSearchLoad', Objects => \@Objects, SearchType => $Type &>
+<& SelectSearchesForObjects, Name => 'SavedSearchLoad', Objects => \@load_objects, SearchType => $Type &>
 <input type="submit" value="<% loc('Load') %>" id="SavedSearchLoadSubmit" name="SavedSearchLoadSubmit" class="button" />
 
 </&>
@@ -93,10 +93,13 @@ my $can_modify = $session{'CurrentUser'}->HasRight(
 );
 
 use RT::SavedSearch;
-my @Objects = RT::SavedSearch->new($session{CurrentUser})->_PrivacyObjects;
-push @Objects, RT::System->new( $session{'CurrentUser'} )
-    if $session{'CurrentUser'}->HasRight( Object=> $RT::System,
-                                          Right => 'SuperUser' );
+my @Objects = RT::SavedSearch->new( $session{CurrentUser} )->ObjectsForModifying( IncludeSuperuserGroups => 0 );
+my @load_objects = RT::SavedSearch->new( $session{CurrentUser} )->ObjectsForLoading( IncludeSuperuserGroups => 0 );
+
+if ( $session{'CurrentUser'}->HasRight( Object => $RT::System, Right  => 'SuperUser' ) ) {
+    push @Objects, RT::System->new( $session{'CurrentUser'} );
+    push @load_objects, RT::System->new( $session{'CurrentUser'} );
+}
 
 my $is_dirty = sub {
     my %arg = (

commit 820bc10de9e5f00a0f8ff5c5c2a7d599b98780bc
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Feb 14 03:00:14 2018 +0800

    update tests for the shared setting access logic change(no group member limit)

diff --git a/t/api/savedsearch.t b/t/api/savedsearch.t
index 2e924bf7b..b33d1e112 100644
--- a/t/api/savedsearch.t
+++ b/t/api/savedsearch.t
@@ -2,7 +2,7 @@ use strict;
 use warnings;
 BEGIN { $ENV{'LANG'} = 'C' }
 
-use RT::Test tests => 27;
+use RT::Test tests => undef;
 
 use_ok('RT::SavedSearch');
 use_ok('RT::SavedSearches');
@@ -86,14 +86,19 @@ my $groupsearch = RT::SavedSearch->new($curruser);
 ok($ret, "groupsearch was created");
 
 my $othersearch = RT::SavedSearch->new($curruser);
-($ret, $msg) = $othersearch->Save(Privacy => 'RT::Group-' . $outgroup->Id,
-                                  Type => 'Ticket',
-                                  Name => 'searchuser requested',
-                                  SearchParams => {'Format' => $format,
-                                                   'Query' => 
-                                                       "Requestor.Name LIKE 'search'"});
+warning_like {
+    ( $ret, $msg ) = $othersearch->Save(
+        Privacy      => 'RT::Group-' . $outgroup->Id,
+        Type         => 'Ticket',
+        Name         => 'searchuser requested',
+        SearchParams => {
+            'Format' => $format,
+            'Query'  => "Requestor.Name LIKE 'search'",
+        }
+    );
+} qr/Permission Denied/;
 ok(!$ret, "othersearch NOT created");
-like($msg, qr/Failed to load object for/, "...for the right reason");
+like($msg, qr/Failed to create search attribute/, "...for the right reason");
 
 $othersearch = RT::SavedSearch->new(RT->SystemUser);
 ($ret, $msg) = $othersearch->Save(Privacy => 'RT::Group-' . $outgroup->Id,
@@ -143,9 +148,9 @@ my $loadedsearch4 = RT::SavedSearch->new($curruser);
 
 warning_like {
     $loadedsearch4->Load($othersearch->Privacy, $othersearch->Id);
-} qr/Could not load object RT::Group-\d+ when loading search/;
+} qr/Deserialization of attribute/, 'load othersearch caused content warning';
 
-isnt($loadedsearch4->Id, $othersearch->Id, "Did not load othersearch");
+is($loadedsearch4->Id, $othersearch->Id, "id is there though");
 
 # Try to update an existing search.
 $loadedsearch1->Update( SearchParams => {'Format' => $format,
@@ -179,3 +184,4 @@ ok($ret, "Deleted genericsearch");
 $allsearches->LimitToPrivacy('RT::User-'.$curruser->Id);
 is($allsearches->Count, 1, "Found all searchuser's searches after deletion");
 
+done_testing;
diff --git a/t/web/dashboards-groups.t b/t/web/dashboards-groups.t
index 9f1c37deb..78847b1f7 100644
--- a/t/web/dashboards-groups.t
+++ b/t/web/dashboards-groups.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Test nodata => 1, tests => 64;
+use RT::Test nodata => 1, tests => undef;
 my ($baseurl, $m) = RT::Test->started_ok;
 
 my $url = $m->rt_base_url;
@@ -26,7 +26,8 @@ $user_obj->PrincipalObj->GrantRight(Right => $_, Object => $queue)
 # are checked and not these as well
 $user_obj->PrincipalObj->GrantRight(Right => $_, Object => $RT::System)
     for qw/SubscribeDashboard CreateOwnDashboard SeeOwnDashboard ModifyOwnDashboard DeleteOwnDashboard/;
-# create and test groups (outer < inner < user)
+
+# create and test groups
 my $inner_group = RT::Group->new(RT->SystemUser);
 ($ok, $msg) = $inner_group->CreateUserDefinedGroup(Name => "inner", Description => "inner group");
 ok($ok, "created inner group: $msg");
@@ -35,22 +36,6 @@ my $outer_group = RT::Group->new(RT->SystemUser);
 ($ok, $msg) = $outer_group->CreateUserDefinedGroup(Name => "outer", Description => "outer group");
 ok($ok, "created outer group: $msg");
 
-($ok, $msg) = $outer_group->AddMember($inner_group->PrincipalId);
-ok($ok, "added inner as a member of outer: $msg");
-
-($ok, $msg) = $inner_group->AddMember($user_obj->PrincipalId);
-ok($ok, "added user as a member of member: $msg");
-
-ok($outer_group->HasMember($inner_group->PrincipalId), "outer has inner");
-ok(!$outer_group->HasMember($user_obj->PrincipalId), "outer doesn't have user directly");
-ok($outer_group->HasMemberRecursively($inner_group->PrincipalId), "outer has inner recursively");
-ok($outer_group->HasMemberRecursively($user_obj->PrincipalId), "outer has user recursively");
-
-ok(!$inner_group->HasMember($outer_group->PrincipalId), "inner doesn't have outer");
-ok($inner_group->HasMember($user_obj->PrincipalId), "inner has user");
-ok(!$inner_group->HasMemberRecursively($outer_group->PrincipalId), "inner doesn't have outer, even recursively");
-ok($inner_group->HasMemberRecursively($user_obj->PrincipalId), "inner has user recursively");
-
 ok $m->login(customer => 'customer'), "logged in";
 
 
@@ -106,7 +91,7 @@ $m->get_ok("/Prefs/DashboardsInMenu.html");
 $m->content_contains("inner dashboard", "Can also see it in the menu options");
 
 my ($group) = grep {$_->isa("RT::Group") and $_->Id == $inner_group->Id}
-    RT::Dashboard->new($currentuser)->_PrivacyObjects;
+    RT::Dashboard->new($currentuser)->ObjectsForLoading;
 ok($group, "Found the group in  the privacy objects list");
 
 my @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
@@ -116,8 +101,7 @@ is_deeply(
     "We can load from ourselves (SeeOwnDashboard) and a group we are with SeeGroupDashboard"
 );
 
-# If you are granted SeeGroupDashboard globally, you can only see
-# dashboards in groups you are in.
+# If you are granted SeeGroupDashboard globally, you can see dashboards in groups.
 $user_obj->PrincipalObj->RevokeRight(
     Right  => 'SeeGroupDashboard',
     Object => $inner_group,
@@ -127,29 +111,27 @@ $user_obj->PrincipalObj->GrantRight(
     Object => RT->System,
 );
 $m->get_ok("/Dashboards/index.html");
-$m->content_contains("inner dashboard", "Having SeeGroupDashboard gobally is fine for groups you are in");
+$m->content_contains("inner dashboard", "Having SeeGroupDashboard gobally also works");
 @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
 is_deeply(
     \@loading,
     ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
-    "SeeGroupDashboard globally still works for groups you are in"
+    "SeeGroupDashboard globally still works for groups"
 );
 
-$inner_group->DeleteMember($user_obj->PrincipalObj->Id);
-ok(!$outer_group->HasMemberRecursively($user_obj->PrincipalId), "outer no longer has user recursively");
-ok(!$inner_group->HasMemberRecursively($user_obj->PrincipalId), "inner no longer has user recursively");
 $m->get_ok("/Dashboards/index.html");
-$m->content_lacks("inner dashboard", "But global SeeGroupDashboard isn't enough for other groups");
+$m->content_contains("inner dashboard", "Global SeeGroupDashboard is enough for all the groups");
 $m->no_warnings_ok;
 @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
 is_deeply(
     \@loading,
-    ["RT::User-".$user_obj->Id],
-    "We only have our SeeOwnDashboard right, as we are no longer in inner"
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id],
+    "We still have group dashboards"
 );
 
-# Similarly, if you're a SuperUser, you still only see dashboards for
-# groups you belong to
+# If you're a SuperUser, you still need global SeeGroupDashboard right to see
+# dashboards in groups
+
 $user_obj->PrincipalObj->RevokeRight(
     Right  => 'SeeGroupDashboard',
     Object => RT->System,
@@ -159,13 +141,7 @@ $user_obj->PrincipalObj->GrantRight(
     Object => RT->System,
 );
 $m->get_ok("/Dashboards/index.html");
-$m->content_lacks("inner dashboard", "Superuser can't see dashboards in groups they're not in");
- at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
-is_deeply(
-    \@loading,
-    ["RT::User-".$user_obj->Id, "RT::System-1"],
-    "We pick up the system-level SeeDashboard right from superuser"
-);
+$m->content_lacks("inner dashboard", "Superuser can't see dashboards in groups without SeeGroupRights");
 @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperuserGroups => 0);
 is_deeply(
     \@loading,
@@ -173,23 +149,27 @@ is_deeply(
     "IncludeSuperusers only cuts out _group_ dashboard objects for loading, not user and system ones"
 );
 
-$inner_group->AddMember($user_obj->PrincipalId);
-$m->get_ok("/Dashboards/index.html");
-$m->content_contains("inner dashboard", "Superuser can see dashboards in groups they are in");
- at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading;
+ at loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperuserGroups => 1);
 is_deeply(
     \@loading,
     ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id, "RT::System-1"],
-    "Becoming a member of the group makes it a possibility"
+    "IncludeSuperuserGroups => 1 returns groups for super user even without SeeGroupDashboard"
+);
+
+$user_obj->PrincipalObj->GrantRight(
+    Right  => 'SeeGroupDashboard',
+    Object => $inner_group,
 );
+$m->get_ok("/Dashboards/index.html");
+$m->content_contains("inner dashboard", "superuser can see dashboards in groups they have SeeGroupDashboard");
 @loading = map {ref($_)."-".$_->Id} RT::Dashboard->new($currentuser)->ObjectsForLoading(IncludeSuperuserGroups => 0);
 is_deeply(
     \@loading,
-    ["RT::User-".$user_obj->Id, "RT::System-1"],
-    "But only via superuser"
+    ["RT::User-".$user_obj->Id, "RT::Group-".$inner_group->Id, "RT::System-1"],
+    "even with IncludeSuperuserGroups => 0"
 );
 
-$m->get_ok("/Dashboards/index.html");
-$m->content_contains("inner dashboard", "The dashboards list includes superuser rights");
 $m->get_ok("/Prefs/DashboardsInMenu.html");
-$m->content_lacks("inner dashboard", "But the menu skips them");
+$m->content_contains("inner dashboard", "can also see it in the menu options");
+
+done_testing;
diff --git a/t/web/saved_search_update.t b/t/web/saved_search_update.t
index f06f70fb6..fa42cb293 100644
--- a/t/web/saved_search_update.t
+++ b/t/web/saved_search_update.t
@@ -2,7 +2,7 @@
 use strict;
 use warnings;
 
-use RT::Test tests => 18;
+use RT::Test tests => undef;
 
 my $root = RT::User->new( $RT::SystemUser );
 $root->Load('root');
@@ -12,7 +12,6 @@ ok( $uid, 'loaded root' );
 my $group = RT::Group->new( $RT::SystemUser );
 my ($gid) = $group->CreateUserDefinedGroup( Name => 'foo' );
 ok( $gid, 'created group foo');
-ok( $group->AddMember( $root->PrincipalId ) );
 
 my ( $baseurl, $m ) = RT::Test->started_ok;
 ok($m->login, 'logged in');
@@ -33,10 +32,30 @@ $m->form_name("BuildQuery");
 is($m->value('SavedSearchDescription'), 'user_saved', "name is correct");
 like($m->value('SavedSearchOwner'), qr/^RT::User-\d+$/, "name is correct");
 ok(
-    scalar grep { $_ eq "RT::Group-$gid" }
-      $m->current_form->find_input('SavedSearchOwner')->possible_values,
+    !( scalar grep { $_ eq "RT::Group-$gid" }
+      $m->current_form->find_input('SavedSearchOwner')->possible_values ),
+    'no group foo'
+);
+
+RT::Test->add_rights(
+    {
+        Principal => $root,
+        Right     => [qw(ShowSavedSearches EditSavedSearches)],
+        Object    => $group,
+    },
+);
+
+$m->reload;
+$m->form_name("BuildQuery");
+
+ok(
+    (
+        scalar grep { $_ eq "RT::Group-$gid" }
+          $m->current_form->find_input( 'SavedSearchOwner' )->possible_values
+    ),
     'found group foo'
 );
+
 $m->field(SavedSearchDescription => 'group_saved');
 $m->select(SavedSearchOwner => "RT::Group-$gid");
 $m->click("SavedSearchSave");
@@ -60,3 +79,103 @@ $m->form_name("BuildQuery");
 is($m->value('SavedSearchOwner'), "RT::System-1", "privacy is correct");
 is($m->value('SavedSearchDescription'), 'system_saved', "name is correct");
 
+# save a group copy of saved search for later super user use
+$m->select( SavedSearchOwner => "RT::Group-$gid" );
+$m->field( SavedSearchDescription => 'copy saved' );
+$m->click( "SavedSearchCopy" );
+$m->form_name("BuildQuery");
+is( $m->value( 'SavedSearchOwner' ), "RT::Group-$gid", "privacy is correct" );
+is( $m->value( 'SavedSearchDescription' ), 'copy saved', "name is correct" );
+
+
+$m->logout;
+
+
+my $user_a = RT::Test->load_or_create_user( Name => 'user_a', Password => 'password', );
+$uid = $user_a->id;
+ok( $uid, 'loaded user_a' );
+
+RT::Test->add_rights(
+    {
+        Principal => $user_a,
+        Right     => [qw(CreateSavedSearch LoadSavedSearch ModifySelf)],
+        Object    => RT->System,
+    },
+);
+
+ok($m->login('user_a', 'password'), 'logged in');
+
+$m->follow_link_ok( { text => "Tickets" }, "to query builder" );
+$m->form_name( "BuildQuery" );
+
+ok(
+    !(
+        grep { $_ eq "RT::Group-$gid" }
+        $m->current_form->find_input( 'SavedSearchLoad' )->possible_values
+    ),
+    'no group foo in load'
+);
+
+ok(
+    !(
+        grep { $_ eq "RT::Group-$gid" }
+        $m->current_form->find_input( 'SavedSearchOwner' )->possible_values
+    ),
+    'no group foo in save'
+);
+
+RT::Test->add_rights(
+    {
+        Principal => $user_a,
+        Right     => [qw(ShowSavedSearches)],
+        Object    => $group,
+    },
+);
+
+$m->reload;
+$m->form_name( "BuildQuery" );
+
+ok(
+    (
+        scalar grep { /RT::Group-$gid/ }
+          $m->current_form->find_input( 'SavedSearchLoad' )->possible_values
+    ),
+    'found group foo in load'
+);
+
+ok(
+    !(
+        scalar grep { $_ eq "RT::Group-$gid" }
+        $m->current_form->find_input( 'SavedSearchOwner' )->possible_values
+    ),
+    'still no group foo in save'
+);
+
+RT::Test->add_rights(
+    {
+        Principal => $user_a,
+        Right     => [qw(EditSavedSearches)],
+        Object    => $group,
+    },
+);
+
+$m->reload;
+$m->form_name( "BuildQuery" );
+
+ok(
+    (
+        scalar grep { /RT::Group-$gid/ }
+          $m->current_form->find_input( 'SavedSearchLoad' )->possible_values
+    ),
+    'found group foo in load'
+);
+
+ok(
+    (
+        scalar grep { $_ eq "RT::Group-$gid" }
+          $m->current_form->find_input( 'SavedSearchOwner' )->possible_values
+    ),
+    'also found group foo in save'
+);
+
+done_testing;

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


More information about the rt-commit mailing list