[Rt-commit] rt branch, 4.4/system-saved-search-delete, created. rt-4.4.4-98-g2b4094d8ce
Jim Brandt
jbrandt at bestpractical.com
Fri Apr 3 17:02:18 EDT 2020
The branch, 4.4/system-saved-search-delete has been created
at 2b4094d8cead01b86c9a4dc7e4401928fb40815d (commit)
- Log -----------------------------------------------------------------
commit cd86eac07c2ced6a35037710c8c2826b7c43a0b7
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Apr 3 16:14:17 2020 -0400
Explicitly check rights when loading RT System saved search
Check rights before trying to load and access the saved
search attribute. Without the check, trying to access
the attribute generates errors like:
Deserialization of attribute 28 failed
diff --git a/share/html/Search/Elements/EditSearches b/share/html/Search/Elements/EditSearches
index 6e7220eb13..6a402f380b 100644
--- a/share/html/Search/Elements/EditSearches
+++ b/share/html/Search/Elements/EditSearches
@@ -160,11 +160,23 @@ if ( $ARGS{'SavedSearchRevert'} ) {
$ARGS{'SavedSearchLoad'} = $SavedSearch->{'Id'};
}
+# See RT::Attribute for mappings of update, delete, display to actual
+# RT rights for the rights checks used here.
+
if ( $ARGS{'SavedSearchLoad'} ) {
my ($container, $id ) = _parse_saved_search ($ARGS{'SavedSearchLoad'});
+
if ( $container ) {
my $search = RT::Attribute->new( $session{'CurrentUser'} );
$search->Load( $id );
+
+ if ($search) {
+ unless ($search->CurrentUserHasRight('display')) {
+ push @results, loc("No permission to load search");
+ return @results;
+ }
+ }
+
$SavedSearch->{'Id'} = $ARGS{'SavedSearchLoad'};
$SavedSearch->{'Object'} = $search;
$SavedSearch->{'Description'} = $search->Description;
commit 03719408420be82a72a964f0db7c4b1ea485d79b
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Thu Apr 2 17:07:47 2020 -0400
Add tests for loading RT System saved searches
diff --git a/t/web/saved_search_permissions.t b/t/web/saved_search_permissions.t
index e24ae6146f..2b3b60eaff 100644
--- a/t/web/saved_search_permissions.t
+++ b/t/web/saved_search_permissions.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => 12;
+use RT::Test tests => undef;
my $user = RT::User->new(RT->SystemUser);
ok(
$user->Create(
@@ -31,3 +31,28 @@ $m->content_contains( $message, 'user foo can not load saved search of root' );
$m->warning_like( qr/User #\d+ tried to load container user #\d+/,
'get warning' );
+
+diag('Test RT System saved searches');
+ok( $m->logout(), 'User foo logged out');
+ok( $m->login(), 'root logged in' );
+$m->get_ok( $url . '/Search/Build.html?Query=id<20' );
+$m->submit_form(
+ form_name => 'BuildQuery',
+ fields => { SavedSearchOwner => 'RT::System-1', SavedSearchDescription => 'Less than 20' },
+ button => 'SavedSearchSave',
+);
+$m->content_contains( q{name="SavedSearchDescription" value="Less than 20"}, 'Saved Less than 20 search' );
+($id) = $m->content =~ /value="(RT::System-1-SavedSearch-\d+)"/;
+
+ok( $m->login( 'foo', 'foobar', logout => 1 ), 'User foo logged in' );
+$m->get_ok( $url . "/Search/Build.html?SavedSearchLoad=$id" );
+
+$message = qq{No permission to load search};
+$m->content_contains( $message, 'user foo can not load RT System system-wide searches' );
+
+ok($user->PrincipalObj->GrantRight(Object => RT->System, Right =>'ShowSavedSearches'),
+ 'Granted foo ShowSavedSearches');
+$m->get_ok( $url . "/Search/Build.html?SavedSearchLoad=$id" );
+$m->content_contains('Loaded saved search', 'User foo loaded RT System saved search' );
+
+done_testing;
commit 743959fd0fbf8a08e86c939d35fe1e5ead3e21ec
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Apr 3 16:37:16 2020 -0400
Explicitly check rights before deleting a saved search
diff --git a/share/html/Search/Elements/EditSearches b/share/html/Search/Elements/EditSearches
index 6a402f380b..7d86f060cb 100644
--- a/share/html/Search/Elements/EditSearches
+++ b/share/html/Search/Elements/EditSearches
@@ -61,7 +61,9 @@
% if ( $Dirty ) {
<input type="submit" class="button" name="SavedSearchRevert" value="<%loc('Revert')%>" />
% }
+% if ( $Object && $Object->Id && $Object->CurrentUserHasRight('delete') ) {
<input type="submit" class="button" name="SavedSearchDelete" value="<%loc('Delete')%>" />
+% }
% if ( $AllowCopy ) {
<input type="submit" class="button" name="SavedSearchCopy" value="<%loc('Save as New')%>" />
% }
@@ -82,6 +84,7 @@
</&>
</div>
<%INIT>
+
return unless $session{'CurrentUser'}->HasRight(
Right => 'LoadSavedSearch',
Object => $RT::System,
@@ -195,8 +198,22 @@ if ( $ARGS{'SavedSearchLoad'} ) {
}
}
elsif ( $ARGS{'SavedSearchDelete'} ) {
- # We set $SearchId to 'new' above already, so peek into the %ARGS
+ # Get the search id from $SavedSearch
my ($container, $id) = _parse_saved_search( $SavedSearch->{'Id'} );
+
+ if ( $container ) {
+ # Load the attribute first to check rights before deleting
+ my $search = RT::Attribute->new( $session{'CurrentUser'} );
+ $search->Load( $id );
+
+ if ($search) {
+ unless ($search->CurrentUserHasRight('delete')) {
+ push @results, loc("No permission to delete search");
+ return @results;
+ }
+ }
+ }
+
if ( $container && $container->id ) {
# We have the object the entry is an attribute on; delete the entry...
my ($val, $msg) = $container->Attributes->DeleteEntry( Name => 'SavedSearch', id => $id );
commit 2b4094d8cead01b86c9a4dc7e4401928fb40815d
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Apr 3 16:39:34 2020 -0400
Add tests for deleting a saved search
diff --git a/t/web/saved_search_permissions.t b/t/web/saved_search_permissions.t
index 2b3b60eaff..fc35cc480e 100644
--- a/t/web/saved_search_permissions.t
+++ b/t/web/saved_search_permissions.t
@@ -50,9 +50,30 @@ $m->get_ok( $url . "/Search/Build.html?SavedSearchLoad=$id" );
$message = qq{No permission to load search};
$m->content_contains( $message, 'user foo can not load RT System system-wide searches' );
+# Grant rights to display the saved search interface on Query Builder
+ok($user->PrincipalObj->GrantRight(Object => RT->System, Right =>'CreateSavedSearch'),
+ 'Granted foo CreateSavedSearch');
+ok($user->PrincipalObj->GrantRight(Object => RT->System, Right =>'LoadSavedSearch'),
+ 'Granted foo LoadSavedSearch');
ok($user->PrincipalObj->GrantRight(Object => RT->System, Right =>'ShowSavedSearches'),
'Granted foo ShowSavedSearches');
$m->get_ok( $url . "/Search/Build.html?SavedSearchLoad=$id" );
$m->content_contains('Loaded saved search', 'User foo loaded RT System saved search' );
+$m->get_ok( $url . "/Search/Build.html?SavedSearchLoad=$id" );
+$m->content_lacks('name="SavedSearchSave"', 'Update button not shown to user foo' );
+$m->content_lacks('name="SavedSearchDelete"', 'Delete button not shown to user foo' );
+
+# Try to delete directly
+$m->get_ok( $url . "/Search/Build.html?SavedSearchDelete=1&SavedSearchId=$id" );
+$message = qq{No permission to delete search};
+$m->content_contains( $message, 'user foo can not delete RT System saved search' );
+
+ok($user->PrincipalObj->GrantRight(Object => RT->System, Right =>'EditSavedSearches'),
+ 'Granted foo EditSavedSearches');
+$m->get_ok( $url . "/Search/Build.html?SavedSearchDelete=1&SavedSearchId=$id" );
+$message = qq{Deleted saved search};
+$m->content_contains( $message, 'user foo deleted RT saved search' );
+
+
done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list