[Rt-commit] rt branch 5.0.5-releng updated. rt-5.0.5beta1-27-g90fb016e60
BPS Git Server
git at git.bestpractical.com
Thu Oct 19 14:54:46 UTC 2023
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".
The branch, 5.0.5-releng has been updated
via 90fb016e604942256edf00a36644ce077bb5ea4e (commit)
via 54a3f830eb4205550692ec6f57c5124acdbf5bf1 (commit)
via 5f76719a96e1a10d19943b70e961859cdd4138b3 (commit)
via 5f3e8b6675cdbc30ca8609707a4f6c11c2ad7fb7 (commit)
via 3b7cfecc2758145e80ea0d6c10998d34c953a2fa (commit)
via 325ddfeda6255f7c011d3c6547ccaff77d9938af (commit)
via c4159088d317827a7dd8ebfd981269e970bcb59a (commit)
via 7b0ff85dd18d0ac5f407edc138034dd5e0b35efd (commit)
via fcf8f76bb5ed19c35c7c5997588feb7b0b556ef2 (commit)
via c2d5d72cd5e6cfaed744a3a2defe716ca96d5375 (commit)
via 666f8b43a7566b8aef4caedf258254eaf0e11586 (commit)
via 96f0305b8515214a22eb2d2b1f23695b81e3c3ae (commit)
via 446479dd140b1af510c75733d48db1524ccdd597 (commit)
via 8032d4ab00fd9ab5d3adfc11671bc37715a7bdd0 (commit)
via 3511c116f0b2fcb917b564f833b02560b981260c (commit)
via 361d2849af8a87795e62bed293ca508d615792f9 (commit)
via 54f25d67378862f6f181f9e4fec0758cc73d2bf0 (commit)
via 4c5de2bcb9cec6fe899890b4af3802697c333ca2 (commit)
via 6091759205c3b4a178cf274c73d66802efe48f03 (commit)
via 0118c94906565a8d4536ef024bf4885fca717011 (commit)
via 0dc3835ffc44e3d34ea14089f76959ca23ae08dc (commit)
via 4dbb8b0797bdc693a95ba95e3e638113f65b63f4 (commit)
via af8015b825be60c4c4f733f7451b753f0240ac21 (commit)
via ef958dadf4387f11817b1125ecd5cbfa973e4d66 (commit)
via f5bfef5be8bf1ca60faa73caea9203d1d239da25 (commit)
via 4cb028365b4ef7524bd1f1192b5c0b5f1f97d66a (commit)
via 2b9ff1a8218bdbf4931fcea8c3676370669e1b24 (commit)
from 1280a7a987154a19ce3de63b2adc1a4735bcb6a3 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 90fb016e604942256edf00a36644ce077bb5ea4e
Merge: 1280a7a987 54a3f830eb
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Oct 19 10:38:31 2023 -0400
Merge branch 'security/5.0.5-releng' into 5.0.5-releng
commit 54a3f830eb4205550692ec6f57c5124acdbf5bf1
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Oct 17 16:30:03 2023 -0400
Fix typo
diff --git a/docs/web_deployment.pod b/docs/web_deployment.pod
index 5d2f3eccb6..d186800bc2 100644
--- a/docs/web_deployment.pod
+++ b/docs/web_deployment.pod
@@ -169,7 +169,7 @@ If you run C<bin/rt-mailgate> on a separate server, you can update
the above to allow additional IP addresses.
<Location /REST/1.0/NoAuth/mail-gateway>
- Require ip 127.0.0.1 ::1 192.0.2.0 # Add you actual IPs
+ Require ip 127.0.0.1 ::1 192.0.2.0 # Add your actual IPs
</Location>
See the L<Apache documentation|https://httpd.apache.org/docs/2.4/mod/mod_authz_host.html>
commit 5f76719a96e1a10d19943b70e961859cdd4138b3
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Oct 12 16:43:33 2023 -0400
Enable devel mode for mailgate tests that depend on detailed output
Now we only show detailed output on devel mode, see also ef958dadf4.
diff --git a/t/mail/gateway.t b/t/mail/gateway.t
index c51daa9092..8f9e941c40 100644
--- a/t/mail/gateway.t
+++ b/t/mail/gateway.t
@@ -2,7 +2,7 @@ use strict;
use warnings;
-use RT::Test config => 'Set( @MailPlugins, "Action::Take", "Action::Resolve");', tests => undef, actual_server => 1;
+use RT::Test config => 'Set( @MailPlugins, "Action::Take", "Action::Resolve"); Set($DevelMode, 1);', tests => undef, actual_server => 1;
my ($baseurl, $m) = RT::Test->started_ok;
use RT::Tickets;
diff --git a/t/mail/han-encodings.t b/t/mail/han-encodings.t
index bc52264961..8bd34e2572 100644
--- a/t/mail/han-encodings.t
+++ b/t/mail/han-encodings.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => undef, actual_server => 1;
+use RT::Test tests => undef, config => 'Set($DevelMode, 1);', actual_server => 1;
# we can't simply call RT::StaticUtil::RequireModule("Encode::HanExtra") here because we are testing
# if Encode::HanExtra could be automatically loaded.
diff --git a/t/ticket/interface.t b/t/ticket/interface.t
index c43e13419e..9067f86967 100644
--- a/t/ticket/interface.t
+++ b/t/ticket/interface.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => undef, actual_server => 1;
+use RT::Test tests => undef, config => 'Set($DevelMode, 1);', actual_server => 1;
my ( $baseurl, $m ) = RT::Test->started_ok;
commit 5f3e8b6675cdbc30ca8609707a4f6c11c2ad7fb7
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 27 12:16:07 2023 -0400
Generate correct SQL for txn search when Owner has ShowTicket
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index dae0d3bf2c..88afccb28e 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -3057,6 +3057,8 @@ sub CurrentUserCanSee {
FIELD => 'Owner',
VALUE => $id,
ENTRYAGGREGATOR => $ea,
+ # RT::Transactions::CurrentUserCanSee reuses RT::Tickets::CurrentUserCanSee
+ ALIAS => $self->isa('RT::Transactions') ? $self->_JoinTickets : 'main',
);
}
else {
commit 3b7cfecc2758145e80ea0d6c10998d34c953a2fa
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 16:55:33 2023 -0400
Update tests as RT-Send-Cc is cleared now
diff --git a/t/mail/sendmail-plaintext.t b/t/mail/sendmail-plaintext.t
index b9eb719516..141039244c 100644
--- a/t/mail/sendmail-plaintext.t
+++ b/t/mail/sendmail-plaintext.t
@@ -132,7 +132,7 @@ for my $encoding ('ISO-8859-1', 'UTF-8') {
{
my ($ticket) = mail_in_ticket('rt-send-cc');
my $cc = first_attach($ticket)->GetHeader('RT-Send-Cc');
- like ($cc, qr/test$_/, "Found test $_") for 1..5;
+ ok (!$cc, "No RT-Send-Cc"); # RT-Send-Cc is supposed to be cleared
}
{
diff --git a/t/mail/sendmail.t b/t/mail/sendmail.t
index 4ef320611b..d6ead4d802 100644
--- a/t/mail/sendmail.t
+++ b/t/mail/sendmail.t
@@ -157,7 +157,7 @@ for my $encoding ('ISO-8859-1', 'UTF-8') {
{
my ($ticket) = mail_in_ticket('rt-send-cc');
my $cc = first_attach($ticket)->GetHeader('RT-Send-Cc');
- like ($cc, qr/test$_/, "Found test $_") for 1..5;
+ ok (!$cc, "No RT-Send-Cc"); # RT-Send-Cc is supposed to be cleared
}
{
commit 325ddfeda6255f7c011d3c6547ccaff77d9938af
Merge: 4c5de2bcb9 c4159088d3
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 16:53:06 2023 -0400
Merge branch 'security/5.0/rest2-stricter-access' into security/5.0.5-releng
commit c4159088d317827a7dd8ebfd981269e970bcb59a
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Sep 20 05:05:31 2022 +0800
Update tests for the total/pages hiding change
See also c2d5d72cd5.
diff --git a/t/rest2/articles.t b/t/rest2/articles.t
index 462a407dd7..3363a34176 100644
--- a/t/rest2/articles.t
+++ b/t/rest2/articles.t
@@ -172,7 +172,7 @@ TODO: {
is( $content->{count}, 2 );
is( $content->{page}, 1 );
is( $content->{per_page}, 20 );
- is( $content->{total}, 2 );
+ is( $content->{total}, undef, 'No total count');
is( scalar @{ $content->{items} }, 2 );
for my $txn ( @{ $content->{items} } ) {
diff --git a/t/rest2/assets.t b/t/rest2/assets.t
index 2685b2dff2..03585d8710 100644
--- a/t/rest2/assets.t
+++ b/t/rest2/assets.t
@@ -256,7 +256,7 @@ my ($asset_url, $asset_id);
is($content->{count}, 3);
is($content->{page}, 1);
is($content->{per_page}, 20);
- is($content->{total}, 3);
+ is($content->{total}, undef, 'No total');
is(scalar @{$content->{items}}, 3);
for my $txn (@{ $content->{items} }) {
diff --git a/t/rest2/attachments.t b/t/rest2/attachments.t
index 416ea504e5..f76baf12b6 100644
--- a/t/rest2/attachments.t
+++ b/t/rest2/attachments.t
@@ -109,8 +109,8 @@ $image_content = MIME::Base64::encode_base64($image_content);
cmp_deeply(
$mech->json_response,
{ per_page => 20,
- pages => 1,
- total => 4,
+ pages => undef,
+ total => undef,
page => 1,
count => 4,
items => [
diff --git a/t/rest2/customfields.t b/t/rest2/customfields.t
index 60d6790e4b..463215731f 100644
--- a/t/rest2/customfields.t
+++ b/t/rest2/customfields.t
@@ -82,7 +82,7 @@ my $freeform_cf_id;
is($res->code, 200);
my $content = $mech->json_response;
- is($content->{total}, 3);
+ is($content->{total}, undef, 'No total');
is($content->{count}, 0);
is_deeply($content->{items}, []);
}
diff --git a/t/rest2/searches.t b/t/rest2/searches.t
index 067ba1fd63..bfae6fdef6 100644
--- a/t/rest2/searches.t
+++ b/t/rest2/searches.t
@@ -65,7 +65,7 @@ ok( $ret, "created $msg" );
is( $content->{count}, 4, '4 searches' );
is( $content->{page}, 1, '1 page' );
is( $content->{per_page}, 20, '20 per_page' );
- is( $content->{total}, 4, '4 total' );
+ is( $content->{total}, undef, 'No total' );
is( scalar @{ $content->{items} }, 4, 'items count' );
for my $item ( @{ $content->{items} } ) {
diff --git a/t/rest2/tickets.t b/t/rest2/tickets.t
index 7c6a378be2..fd563c82e7 100644
--- a/t/rest2/tickets.t
+++ b/t/rest2/tickets.t
@@ -396,9 +396,7 @@ my ($ticket_url, $ticket_id);
is($content->{page}, 1);
is($content->{per_page}, 20);
- # TODO This 14 VS 15 inconsitency is because user lacks ShowOutgoingEmail.
- # It'll be perfect if we can keep them in sync
- is($content->{total}, 15);
+ is($content->{total}, undef, 'No total');
is(scalar @{$content->{items}}, 14);
for my $txn (@{ $content->{items} }) {
diff --git a/t/rest2/transactions.t b/t/rest2/transactions.t
index a2f0a61cd6..fb5b13fbce 100644
--- a/t/rest2/transactions.t
+++ b/t/rest2/transactions.t
@@ -57,7 +57,7 @@ my ($delete_link1_txn_url, $delete_link1_txn_id, $delete_link2_txn_url, $delete_
is($content->{count}, 10);
is($content->{page}, 1);
is($content->{per_page}, 20);
- is($content->{total}, 10);
+ is($content->{total}, undef, 'No total');
is(scalar @{$content->{items}}, 10);
my (
@@ -103,7 +103,7 @@ my ($delete_link1_txn_url, $delete_link1_txn_id, $delete_link2_txn_url, $delete_
is($content->{count}, 10);
is($content->{page}, 1);
is($content->{per_page}, 20);
- is($content->{total}, 10);
+ is($content->{total}, undef, 'No total');
is(scalar @{$content->{items}}, 10);
my (
commit 7b0ff85dd18d0ac5f407edc138034dd5e0b35efd
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Sep 20 05:18:22 2022 +0800
Update tests for the "forbidden" method improvement
See also 666f8b43a7.
diff --git a/t/rest2/cf-image.t b/t/rest2/cf-image.t
index f311c7abec..5025ddf2b4 100644
--- a/t/rest2/cf-image.t
+++ b/t/rest2/cf-image.t
@@ -47,7 +47,7 @@ $user->PrincipalObj->GrantRight( Right => 'SeeCustomField' );
my $res = $mech->get("$rest_base_path/download/cf/666",
'Authorization' => $auth,
);
- is($res->code, 404);
+ is($res->code, 403);
}
# Download cf text
diff --git a/t/rest2/group-members.t b/t/rest2/group-members.t
index 3ceeab8fb5..58c3e675e5 100644
--- a/t/rest2/group-members.t
+++ b/t/rest2/group-members.t
@@ -59,19 +59,12 @@ $group2->Load($group2_id);
);
is($res->code, 403, 'Cannot disable group without AdminGroup right');
- # Rights Test - With AdminGroup, no SeeGroup
+ # Rights Test - With AdminGroup
$user->PrincipalObj->GrantRight(Right => 'AdminGroup', Object => $group2);
$res = $mech->delete($group2_url,
'Authorization' => $auth,
);
- is($res->code, 403, 'Cannot disable group without SeeGroup right');
-
- # Rights Test - With AdminGroup, no SeeGroup
- $user->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $group2);
- $res = $mech->delete($group2_url,
- 'Authorization' => $auth,
- );
- is($res->code, 204, 'Disable group with AdminGroup & SeeGroup rights');
+ is($res->code, 204, 'Disable group with AdminGroup rights');
is($group2->Disabled, 1, "Group disabled");
}
@@ -91,19 +84,12 @@ $group2->Load($group2_id);
'Authorization' => $auth);
is($res->code, 403, 'Cannot enable group without AdminGroup right');
- # Rights Test - With AdminGroup, no SeeGroup
+ # Rights Test - With AdminGroup
$user->PrincipalObj->GrantRight(Right => 'AdminGroup', Object => $group2);
$res = $mech->put_json($group2_url,
$payload,
'Authorization' => $auth);
- is($res->code, 403, 'Cannot enable group without SeeGroup right');
-
- # Rights Test - With AdminGroup, no SeeGroup
- $user->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $group2);
- $res = $mech->put_json($group2_url,
- $payload,
- 'Authorization' => $auth);
- is($res->code, 200, 'Enable group with AdminGroup & SeeGroup rights');
+ is($res->code, 200, 'Enable group with AdminGroup rights');
is_deeply($mech->json_response, ['Group enabled']);
$group2->Load( $group2->Id ); # Reload to refresh $group2->PrincipalObj
@@ -112,7 +98,7 @@ $group2->Load($group2_id);
my $group1_id = $group1->id;
(my $group1_url = $group2_url) =~ s/$group2_id/$group1_id/;
-$user->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $group1);
+$user->PrincipalObj->GrantRight(Right => 'SeeGroup', Object => $_) for $group1, $group2;
# Members addition
{
commit fcf8f76bb5ed19c35c7c5997588feb7b0b556ef2
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Sep 8 21:54:09 2022 +0800
Require SeeGroup to view group transactions
This is to protect group history, which doesn't need to be globally
visible.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index afa8465f6f..4d666f5ba1 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1311,15 +1311,20 @@ 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.
+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.
+
+We do require "SeeGroup" to see transactions of current group.
=cut
sub CurrentUserCanSee {
my $self = shift;
- return 1;
+ my ($what, $txn) = @_;
+
+ return 1 if ( $what // '' ) ne 'Transaction';
+ return $self->CurrentUserHasRight('SeeGroup');
}
=head2 CurrentUserCanCreate
commit c2d5d72cd5e6cfaed744a3a2defe716ca96d5375
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Sep 17 20:45:06 2022 +0800
Hide total/pages info from REST2 collection endpoints without proper rights
As most collection searches do ACL filter after SQL, CountAll returns
the number of results before ACL filter, the number is inaccurate and
can cause info leakage via intentional search criteria.
This commit sets total/pages to null, when user doesn't have proper
right granted globally, to get around this issue.
diff --git a/lib/RT/Articles.pm b/lib/RT/Articles.pm
index 2eb1743ee9..6b40390017 100644
--- a/lib/RT/Articles.pm
+++ b/lib/RT/Articles.pm
@@ -975,6 +975,11 @@ sub SimpleSearch {
return $self;
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'ShowArticle', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Assets.pm b/lib/RT/Assets.pm
index f6cf8ee647..516869f9cc 100644
--- a/lib/RT/Assets.pm
+++ b/lib/RT/Assets.pm
@@ -1932,6 +1932,11 @@ sub _ProcessRestrictions {
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'ShowAsset', Object => RT->System ) ? 1 : 0;
+}
+
1;
RT::Base->_ImportOverlays();
diff --git a/lib/RT/Catalogs.pm b/lib/RT/Catalogs.pm
index 250793c47b..6d67a47720 100644
--- a/lib/RT/Catalogs.pm
+++ b/lib/RT/Catalogs.pm
@@ -114,6 +114,11 @@ sub _Init {
sub Table { "Catalogs" }
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'ShowCatalog', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Classes.pm b/lib/RT/Classes.pm
index ff446dddfb..9cac032d48 100644
--- a/lib/RT/Classes.pm
+++ b/lib/RT/Classes.pm
@@ -81,6 +81,11 @@ sub AddRecord {
sub _SingularClass { "RT::Class" }
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'SeeClass', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/CustomFields.pm b/lib/RT/CustomFields.pm
index 253513d0f9..0fc5569adb 100644
--- a/lib/RT/CustomFields.pm
+++ b/lib/RT/CustomFields.pm
@@ -475,6 +475,11 @@ sub LimitToCatalog {
}
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'SeeCustomField', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/CustomRoles.pm b/lib/RT/CustomRoles.pm
index 6cac676858..75587c71ed 100644
--- a/lib/RT/CustomRoles.pm
+++ b/lib/RT/CustomRoles.pm
@@ -213,6 +213,12 @@ sub LimitToAdded {
return RT::ObjectCustomRoles->new( $self->CurrentUser )->LimitTargetToAdded( $self => @_ );
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ # Not typo, user needs SeeQueue to see CustomRoles
+ return $self->CurrentUser->HasRight( Right => 'SeeQueue', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 2f341bbba3..51a43f95e3 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -537,6 +537,11 @@ sub SimpleSearch {
return $self;
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'SeeGroup', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Queues.pm b/lib/RT/Queues.pm
index 5a916d70a0..0d031b8ebe 100644
--- a/lib/RT/Queues.pm
+++ b/lib/RT/Queues.pm
@@ -128,6 +128,11 @@ sub ItemsOrderBy {
return $items;
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'SeeQueue', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/REST2/Resource/Collection.pm b/lib/RT/REST2/Resource/Collection.pm
index 2f0f16eab2..7653d5695c 100644
--- a/lib/RT/REST2/Resource/Collection.pm
+++ b/lib/RT/REST2/Resource/Collection.pm
@@ -189,7 +189,7 @@ sub serialize {
my %results = (
count => scalar(@results) + 0,
- total => $collection->CountAll + 0,
+ total => $collection->CurrentUserCanSeeAll ? ( $collection->CountAll + 0 ) : undef,
per_page => $collection->RowsPerPage + 0,
page => ($collection->FirstRow / $collection->RowsPerPage) + 1,
items => \@results,
@@ -205,18 +205,20 @@ sub serialize {
}
}
- $results{pages} = ceil($results{total} / $results{per_page});
- if ($results{page} < $results{pages}) {
- my $page = $results{page} + 1;
- $uri->query_form( @query_form, page => $results{page} + 1 );
- $results{next_page} = $uri->as_string;
- };
- if ($results{page} > 1) {
- # If we're beyond the last page, set prev_page as the last page
- # available, otherwise, the previous page.
- $uri->query_form( @query_form, page => ($results{page} > $results{pages} ? $results{pages} : $results{page} - 1) );
- $results{prev_page} = $uri->as_string;
- };
+ $results{pages} = defined $results{total} ? ceil($results{total} / $results{per_page}) : undef;
+ if ( $results{pages} ) {
+ if ($results{page} < $results{pages}) {
+ my $page = $results{page} + 1;
+ $uri->query_form( @query_form, page => $results{page} + 1 );
+ $results{next_page} = $uri->as_string;
+ }
+ if ($results{page} > 1) {
+ # If we're beyond the last page, set prev_page as the last page
+ # available, otherwise, the previous page.
+ $uri->query_form( @query_form, page => ($results{page} > $results{pages} ? $results{pages} : $results{page} - 1) );
+ $results{prev_page} = $uri->as_string;
+ }
+ }
return \%results;
}
diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index adb1e4e953..03088d9394 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -1150,6 +1150,11 @@ sub DistinctFieldValues {
return @values;
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->HasRight( Right => 'SuperUser', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index a62b7de6a1..5f6f3e5487 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -3716,6 +3716,12 @@ sub Query {
return $self->{_sql_query};
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return 1 if RT->Config->Get('UseSQLForACLChecks');
+ return $self->CurrentUser->HasRight( Right => 'ShowTicket', Object => RT->System ) ? 1 : 0;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 33bbbac1f9..16297e4ddf 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -715,6 +715,11 @@ sub SimpleSearch {
return $self;
}
+sub CurrentUserCanSeeAll {
+ my $self = shift;
+ return $self->CurrentUser->Privileged;
+}
+
RT::Base->_ImportOverlays();
1;
commit 666f8b43a7566b8aef4caedf258254eaf0e11586
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Sep 16 03:58:03 2022 +0800
Improve "forbidden" for REST2 record endpoints with more right checks.
Previously returned "not found" if the corresponding record doesn't
exist, even when user doesn't have proper rights granted, which led to
infoleakage.
This commit tweaks the logic to check "CurrentUserCanCreate" if the
record doesn't exist, which eliminates the leakage. Besides that, it
checks "CurrentUserCanModify" instead on object update, which is more
appopriate than "CurrentUserCanSee".
There is a couple of right changes for "/customfield/" and "/group"
endpoints: previously modifying them required "SeeCustomField" and
"SeeGroup", respectively, which was not quite necessary as it's really
odd to grant users "Admin..." but not "See..." rights. This commit
dropped the "SeeCustomField"/"SeeGroup" check, to be consistent with
other objects.
diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 3b6379dd5b..77a5f65ce5 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1092,6 +1092,17 @@ sub _CacheConfig {
}
+=head2 CurrentUserCanSee
+
+Returns true if the current user can see the attachment, via corresponding
+transaction's rights check.
+
+=cut
+
+sub CurrentUserCanSee {
+ my $self = shift;
+ return $self->TransactionObj->CurrentUserCanSee;
+}
=head2 id
diff --git a/lib/RT/Catalog.pm b/lib/RT/Catalog.pm
index 1354207523..31946435b1 100644
--- a/lib/RT/Catalog.pm
+++ b/lib/RT/Catalog.pm
@@ -297,6 +297,28 @@ sub CurrentUserCanSee {
|| $self->CurrentUserHasRight('AdminCatalog');
}
+=head2 CurrentUserCanCreate
+
+Returns true if the current user can create a new catalog, using I<AdminCatalog>.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminCatalog');
+}
+
+=head2 CurrentUserCanModify
+
+Returns true if the current user can modify the catalog, using I<AdminCatalog>.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminCatalog');
+}
+
=head2 Owner
Returns an L<RT::User> object for this catalog's I<Owner> role group. On error,
diff --git a/lib/RT/Class.pm b/lib/RT/Class.pm
index 01079040c0..03a7926cc2 100644
--- a/lib/RT/Class.pm
+++ b/lib/RT/Class.pm
@@ -507,6 +507,39 @@ sub IncludeArticleCFValue {
return $self->{'_cf_include_hash'}{"Value-".$cfobj->Id};
}
+=head2 CurrentUserCanSee
+
+Returns true if the current user can see the class, using I<SeeClass>.
+
+=cut
+
+sub CurrentUserCanSee {
+ my $self = shift;
+ return $self->CurrentUserHasRight('SeeClass');
+}
+
+=head2 CurrentUserCanCreate
+
+Returns true if the current user can create a new class, using I<AdminClass>.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminClass');
+}
+
+=head2 CurrentUserCanModify
+
+Returns true if the current user can modify the class, using I<AdminClass>.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminClass');
+}
+
=head2 id
Returns the current value of id.
diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 79c1f1f5a5..e261eb6f8e 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -2072,6 +2072,28 @@ sub CurrentUserCanSee {
return 0;
}
+=head2 CurrentUserCanCreate
+
+If the user has I<AdminCustomField> they can create a new custom field.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminCustomField');
+}
+
+=head2 CurrentUserCanModify
+
+If the user has I<AdminCustomField> they can modify the custom field.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminCustomField');
+}
+
=head2 IncludeContentForValue [VALUE] (and SetIncludeContentForValue)
Gets or sets the C<IncludeContentForValue> for this custom field. RT
diff --git a/lib/RT/CustomRole.pm b/lib/RT/CustomRole.pm
index ec37402925..750aaf2575 100644
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@ -544,6 +544,28 @@ sub GroupType {
return 'RT::CustomRole-' . $self->id;
}
+=head2 CurrentUserCanCreate
+
+Returns true if the current user can create a new custom role, using I<AdminCustomRoles>.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminClass');
+}
+
+=head2 CurrentUserCanModify
+
+Returns true if the current user can modify the custom role, using I<AdminCustomRoles>.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminClass');
+}
+
=head2 id
Returns the current value of id.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index fd592835bd..afa8465f6f 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1322,6 +1322,27 @@ sub CurrentUserCanSee {
return 1;
}
+=head2 CurrentUserCanCreate
+
+Returns true if the current user can create a new group, using I<AdminGroup>.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminGroup');
+}
+
+=head2 CurrentUserCanModify
+
+Returns true if the current user can modify the group, using I<AdminGroup>.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminGroup');
+}
=head2 PrincipalObj
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index ffad1c4ddd..148bd1c61e 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -823,6 +823,7 @@ object, otherwise false.
sub CurrentUserCanSee {
my $self = shift;
+ return undef unless $self->Id;
return $self->CustomFieldObj->CurrentUserHasRight('SeeCustomField');
}
diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 29ba7db3ad..609a7ebe5b 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -810,6 +810,29 @@ sub CurrentUserCanSee {
return $self->CurrentUserHasRight('SeeQueue');
}
+
+=head2 CurrentUserCanCreate
+
+Returns true if the current user can create a new queue, using I<AdminQueue>.
+
+=cut
+
+sub CurrentUserCanCreate {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminQueue');
+}
+
+=head2 CurrentUserCanModify
+
+Returns true if the current user can modify the queue, using I<AdminQueue>.
+
+=cut
+
+sub CurrentUserCanModify {
+ my $self = shift;
+ return $self->CurrentUserHasRight('AdminQueue');
+}
+
=head2 id
Returns the current value of id.
diff --git a/lib/RT/REST2/Resource/CustomField.pm b/lib/RT/REST2/Resource/CustomField.pm
index 62e2d737b7..1924a95f2f 100644
--- a/lib/RT/REST2/Resource/CustomField.pm
+++ b/lib/RT/REST2/Resource/CustomField.pm
@@ -87,21 +87,6 @@ sub serialize {
return $data;
}
-sub forbidden {
- my $self = shift;
- my $method = $self->request->method;
- if ($self->record->id) {
- if ($method eq 'GET') {
- return !$self->record->CurrentUserHasRight('SeeCustomField');
- } else {
- return !($self->record->CurrentUserHasRight('SeeCustomField') && $self->record->CurrentUserHasRight('AdminCustomField'));
- }
- } else {
- return !$self->current_user->HasRight(Right => "AdminCustomField", Object => RT->System);
- }
- return 0;
-}
-
sub hypermedia_links {
my $self = shift;
my $links = $self->_default_hypermedia_links(@_);
diff --git a/lib/RT/REST2/Resource/Group.pm b/lib/RT/REST2/Resource/Group.pm
index 58f46c1255..c955d1558f 100644
--- a/lib/RT/REST2/Resource/Group.pm
+++ b/lib/RT/REST2/Resource/Group.pm
@@ -58,9 +58,7 @@ extends 'RT::REST2::Resource::Record';
with 'RT::REST2::Resource::Record::Readable'
=> { -alias => { serialize => '_default_serialize' } },
'RT::REST2::Resource::Record::DeletableByDisabling',
- => { -alias => { delete_resource => '_delete_resource' } },
'RT::REST2::Resource::Record::Writable',
- => { -alias => { create_record => '_create_record' } },
'RT::REST2::Resource::Record::Hypermedia'
=> { -alias => { hypermedia_links => '_default_hypermedia_links' } };
@@ -102,33 +100,20 @@ sub hypermedia_links {
return $links;
}
-sub create_record {
+override forbidden => sub {
my $self = shift;
- my $data = shift;
- return (\403, $self->record->loc("Permission Denied"))
- unless $self->current_user->HasRight(
- Right => "AdminGroup",
- Object => RT->System,
- );
-
- return $self->_create_record($data);
-}
-
-sub delete_resource {
- my $self = shift;
-
- return (\403, $self->record->loc("Permission Denied"))
- unless $self->record->CurrentUserHasRight('AdminGroup');
-
- return $self->_delete_resource;
-}
-
-sub forbidden {
- my $self = shift;
- return 0 unless $self->record->id;
- return !$self->record->CurrentUserHasRight('SeeGroup');
-}
+ # For historical reasons, RT::Group::CurrentUserCanSee always returns true.
+ # For REST2, we want to check SeeGroup.
+ no warnings 'redefine';
+ my $original_can_see = \&RT::Group::CurrentUserCanSee;
+ local *RT::Group::CurrentUserCanSee = sub {
+ my $self = shift;
+ return 0 unless $original_can_see->($self, @_);
+ return $self->CurrentUserHasRight('SeeGroup');
+ };
+ return super();
+};
__PACKAGE__->meta->make_immutable;
diff --git a/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm b/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
index 8d36cb9d30..4dbc9cb7a8 100644
--- a/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
+++ b/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
@@ -71,11 +71,6 @@ sub content_types_provided {
{ [ {$self->record->ContentType || 'text/plain; charset=utf-8' => 'to_binary'} ] };
}
-sub forbidden {
- my $self = shift;
- return !$self->record->CurrentUserHasRight('SeeCustomField');
-}
-
sub to_binary {
my $self = shift;
unless ($self->record->CustomFieldObj->Type =~ /^(?:Image|Binary)$/) {
diff --git a/lib/RT/REST2/Resource/Record.pm b/lib/RT/REST2/Resource/Record.pm
index b335dab09e..df1223e993 100644
--- a/lib/RT/REST2/Resource/Record.pm
+++ b/lib/RT/REST2/Resource/Record.pm
@@ -100,10 +100,21 @@ sub resource_exists {
sub forbidden {
my $self = shift;
- return 0 unless $self->record->id;
+ my $method = $self->request->method;
+
+ my $right_method;
+ if ( $self->record->id ) {
+ $right_method = $method =~ /^(?:GET|HEAD)$/ ? 'CurrentUserCanSee' : 'CurrentUserCanModify';
+ }
+ else {
+ # Even without id, the method can be GET, e.g. to access a not-exsting record.
+ $right_method = $method =~ /^(?:GET|HEAD)$/ ? 'CurrentUserCanSee' : 'CurrentUserCanCreate';
+ }
+
+ if ( $self->record->can($right_method) ) {
+ return !$self->record->$right_method;
+ }
- my $can_see = $self->record->can("CurrentUserCanSee");
- return 1 if $can_see and not $self->record->$can_see();
return 0;
}
commit 96f0305b8515214a22eb2d2b1f23695b81e3c3ae
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Sep 17 08:04:14 2022 +0800
Clean up the forbidden method of REST2 "/download/cf/*" endpoint
The code "return 0 unless $self->record->id" is usually to allow to
create the corresponding object, which isn't applicable here as the
endpoint doesn't support custom field creation.
diff --git a/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm b/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
index b9d6fa1cb3..8d36cb9d30 100644
--- a/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
+++ b/lib/RT/REST2/Resource/ObjectCustomFieldValue.pm
@@ -73,7 +73,6 @@ sub content_types_provided {
sub forbidden {
my $self = shift;
- return 0 unless $self->record->id;
return !$self->record->CurrentUserHasRight('SeeCustomField');
}
commit 446479dd140b1af510c75733d48db1524ccdd597
Author: sunnavy <sunnavy at bestpractical.com>
Date: Sat Sep 17 07:50:43 2022 +0800
Clean up the forbidden method of REST2 "/group/.../member*" endpoint
It's fine to deny the access if the targeted group doesn't exist.
The code "return 0 unless $self->group->id" is usually to allow to
create the corresponding object, which isn't applicable here as the
endpoint doesn't support group creation.
diff --git a/lib/RT/REST2/Resource/GroupMembers.pm b/lib/RT/REST2/Resource/GroupMembers.pm
index 4c0bb0a49a..df555babec 100644
--- a/lib/RT/REST2/Resource/GroupMembers.pm
+++ b/lib/RT/REST2/Resource/GroupMembers.pm
@@ -114,9 +114,7 @@ sub dispatch_rules {
sub forbidden {
my $self = shift;
- return 0 unless $self->group->id;
return !$self->group->CurrentUserHasRight('AdminGroupMembership');
- return 1;
}
sub serialize {
commit 8032d4ab00fd9ab5d3adfc11671bc37715a7bdd0
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Sep 13 22:16:15 2022 +0800
Disallow unprivileged users to access REST2 "/user" endpoint
The only exception is current user's own endpoint.
Previously unprivileged users got "forbidden" if the target user exists
and "not found" if the user does not, via which they could info all user
names, which led to info leakage.
diff --git a/lib/RT/REST2/Resource/User.pm b/lib/RT/REST2/Resource/User.pm
index 510a8c7740..af4c8c0c2b 100644
--- a/lib/RT/REST2/Resource/User.pm
+++ b/lib/RT/REST2/Resource/User.pm
@@ -105,9 +105,8 @@ around 'serialize' => sub {
sub forbidden {
my $self = shift;
- return 0 if not $self->record->id;
- return 0 if $self->record->id == $self->current_user->id;
return 0 if $self->current_user->Privileged;
+ return 0 if ( $self->record->id || 0 ) == $self->current_user->id;
return 1;
}
commit 3511c116f0b2fcb917b564f833b02560b981260c
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Sep 16 22:54:14 2022 +0800
Protect queue/class/catalog info on ticket/article/asset create via REST2
Previously we showed "Invalid ..." if the passed queue/class/catalog
argument doesn't exist, via which user can infer all queue/class/catalog
names.
This commit changes it to "Permission Denied", to get around the
possiblity of info leakage.
diff --git a/lib/RT/REST2/Resource/Article.pm b/lib/RT/REST2/Resource/Article.pm
index f3a0eb32d7..d52fff739d 100644
--- a/lib/RT/REST2/Resource/Article.pm
+++ b/lib/RT/REST2/Resource/Article.pm
@@ -80,17 +80,11 @@ sub create_record {
return (\400, "Invalid Class") if !$data->{Class};
- my $class = RT::Class->new(RT->SystemUser);
+ my $class = RT::Class->new($self->current_user);
$class->Load($data->{Class});
- return (\400, "Invalid Class") if !$class->Id;
-
return ( \403, $self->record->loc("Permission Denied") )
- unless $self->record->CurrentUser->HasRight(
- Right => 'CreateArticle',
- Object => $class,
- )
- and $class->Disabled != 1;
+ unless $class->Id and !$class->__Value('Disabled') and $class->CurrentUserHasRight('CreateArticle');
my ($ok, $txn, $msg) = $self->_create_record($data);
return ($ok, $msg);
diff --git a/lib/RT/REST2/Resource/Asset.pm b/lib/RT/REST2/Resource/Asset.pm
index a29f637b7c..70e66464ba 100644
--- a/lib/RT/REST2/Resource/Asset.pm
+++ b/lib/RT/REST2/Resource/Asset.pm
@@ -83,10 +83,8 @@ sub create_record {
my $catalog = RT::Catalog->new($self->record->CurrentUser);
$catalog->Load($data->{Catalog});
- return (\400, "Invalid Catalog") if !$catalog->Id;
-
- return (\403, $self->record->loc("Permission Denied", $catalog->Name))
- unless $catalog->CurrentUserHasRight('CreateAsset');
+ return (\403, $self->record->loc("Permission Denied"))
+ unless $catalog->Id and !$catalog->__Value('Disabled') and $catalog->CurrentUserHasRight('CreateAsset');
return $self->_create_record($data);
}
diff --git a/lib/RT/REST2/Resource/Ticket.pm b/lib/RT/REST2/Resource/Ticket.pm
index 1873f0a636..64b4d67647 100644
--- a/lib/RT/REST2/Resource/Ticket.pm
+++ b/lib/RT/REST2/Resource/Ticket.pm
@@ -225,16 +225,11 @@ sub validate_input {
if ( $args{'Action'} eq 'create' ) {
return (0, "Could not create ticket. Queue not set", 400) if !$data->{Queue};
- my $queue = RT::Queue->new(RT->SystemUser);
+ my $queue = RT::Queue->new($self->current_user);
$queue->Load($data->{Queue});
- return (0, "Unable to find queue", 400) if !$queue->Id;
-
- return (0, $self->record->loc("No permission to create tickets in the queue '[_1]'", $queue->Name), 403)
- unless $self->record->CurrentUser->HasRight(
- Right => 'CreateTicket',
- Object => $queue,
- ) and $queue->Disabled != 1;
+ return (0, $self->record->loc("No permission to create tickets in the queue '[_1]'", $data->{Queue}), 403)
+ unless $queue->Id and $queue->__Value('Disabled') != 1 and $queue->CurrentUserHasRight('CreateTicket');
}
if ( $args{'Action'} eq 'update' ) {
commit 361d2849af8a87795e62bed293ca508d615792f9
Author: sunnavy <sunnavy at bestpractical.com>
Date: Fri Sep 16 01:21:07 2022 +0800
Show Plugins info in "/rt" REST2 endpoint for super users only
Normal users are not supposed to know about it.
diff --git a/lib/RT/REST2/Resource/RT.pm b/lib/RT/REST2/Resource/RT.pm
index 6a31ba3a71..724880af72 100644
--- a/lib/RT/REST2/Resource/RT.pm
+++ b/lib/RT/REST2/Resource/RT.pm
@@ -71,7 +71,9 @@ sub to_json {
my $self = shift;
return JSON::to_json({
Version => $RT::VERSION,
- Plugins => [ RT->Config->Get('Plugins') ],
+ $self->current_user->HasRight( Object => RT->System, Right => 'SuperUser' )
+ ? ( Plugins => [ RT->Config->Get('Plugins') ] )
+ : (),
}, { pretty => 1 });
}
__PACKAGE__->meta->make_immutable;
commit 54f25d67378862f6f181f9e4fec0758cc73d2bf0
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Sep 13 22:02:08 2022 +0800
Hide time fields in REST2 if $HideTimeFieldsFromUnprivilegedUsers is true
This is to sync with web UI.
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 2f615bdcb5..6cdb5f1b9e 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -3786,6 +3786,10 @@ sub Serialize {
$store{EffectiveId} = \( join '-', 'RT::Ticket', $RT::Organization, $store{EffectiveId} );
+ unless ( $self->CurrentUserCanSeeTime ) {
+ delete $store{$_} for qw/TimeEstimated TimeLeft TimeWorked/;
+ }
+
return %store;
}
commit 4c5de2bcb9cec6fe899890b4af3802697c333ca2
Merge: 0dc3835ffc 6091759205
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 15:33:19 2023 -0400
Merge branch 'security/5.0/protect-txn-searches' into security/5.0.5-releng
commit 6091759205c3b4a178cf274c73d66802efe48f03
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Sep 8 15:36:33 2022 +0800
Disallow non-ticket transaction searches from web UI
Currently only ticket transaction searches are supported via web UI, but
user can manually specify ObjectType in URL whatever they like to search
various transactions, which is a bit risky, especially considering some
records don't have appropriate protection setup(like Groups).
Besides, by explicitly limiting searches to be ticket only, the search
count is more accurate(when UseSQLForACLChecks is enabled), which can
reduce the possibility of info disclosure.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 64ce8dc643..5ad8be2928 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -5792,6 +5792,28 @@ sub ParseCalendarData {
return undef;
}
+sub PreprocessTransactionSearchQuery {
+ my %args = (
+ Query => undef,
+ ObjectType => 'RT::Ticket',
+ @_
+ );
+
+ my @limits;
+ if ( $args{ObjectType} eq 'RT::Ticket' ) {
+ @limits = (
+ q{TicketType = 'ticket'},
+ qq{ObjectType = '$args{ObjectType}'},
+ $args{Query} =~ /^\s*\(.*\)$/ ? $args{Query} : "($args{Query})"
+ );
+ }
+ else {
+ # Other ObjectTypes are not supported for now
+ @limits = 'id = 0';
+ }
+ return join ' AND ', @limits;
+}
+
package RT::Interface::Web;
RT::Base->_ImportOverlays();
diff --git a/share/html/Elements/CollectionList b/share/html/Elements/CollectionList
index 2895d2cc83..2f6aeceeb9 100644
--- a/share/html/Elements/CollectionList
+++ b/share/html/Elements/CollectionList
@@ -49,11 +49,7 @@
if (!$Collection) {
$Collection = $Class->new( $session{'CurrentUser'} );
if ( $Class eq 'RT::Transactions' ) {
- my @limits = ( "ObjectType = '$ObjectType'", $Query ? "($Query)" : () );
- if ( $ObjectType eq 'RT::Ticket' ) {
- unshift @limits, "TicketType = 'ticket'";
- }
- $Query = join ' AND ', @limits;
+ $Query = PreprocessTransactionSearchQuery( Query => $Query, ObjectType => $ObjectType );
}
$Collection->FromSQL($Query);
}
diff --git a/share/html/Search/Results.html b/share/html/Search/Results.html
index 53d901f3ad..eb37986b3f 100644
--- a/share/html/Search/Results.html
+++ b/share/html/Search/Results.html
@@ -177,15 +177,9 @@ $session{$session_name} = $Class->new($session{'CurrentUser'}) ;
my ( $ok, $msg );
if ( $Query ) {
if ( $Class eq 'RT::Transactions' ) {
- my @limits = ( "ObjectType = '$ObjectType'", "($Query)" );
- if ( $ObjectType eq 'RT::Ticket' ) {
- unshift @limits, "TicketType = 'ticket'";
- }
- ( $ok, $msg ) = $session{$session_name}->FromSQL( join ' AND ', @limits );
- }
- else {
- ( $ok, $msg ) = $session{$session_name}->FromSQL($Query);
+ $Query = PreprocessTransactionSearchQuery( Query => $Query, ObjectType => $ObjectType );
}
+ ( $ok, $msg ) = $session{$session_name}->FromSQL($Query);
}
# Provide an empty search if parsing failed
diff --git a/share/html/Search/Results.tsv b/share/html/Search/Results.tsv
index 6a3bfc442b..cddb99dfc9 100644
--- a/share/html/Search/Results.tsv
+++ b/share/html/Search/Results.tsv
@@ -61,17 +61,11 @@ my $collection = $Class->new( $session{'CurrentUser'} );
my @limits;
if ( $Class eq 'RT::Transactions' ) {
- @limits = ( "ObjectType = '$ObjectType'", "($Query)" );
- if ( $ObjectType eq 'RT::Ticket' ) {
- unshift @limits, "TicketType = 'ticket'";
- }
-}
-else {
- push @limits, $Query;
+ $Query = PreprocessTransactionSearchQuery( Query => $Query, ObjectType => $ObjectType );
}
if ( $Query ) {
- $collection->FromSQL( join ' AND ', @limits );
+ $collection->FromSQL( $Query );
}
elsif ( $Class eq 'RT::Assets' ) {
my $catalog_obj = LoadDefaultCatalog($ARGS{'Catalog'} || '');
commit 0118c94906565a8d4536ef024bf4885fca717011
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 7 21:53:34 2022 +0800
Support UseSQLForACLChecks for ticket transaction searches
This makes ticket transaction search count more accurate. Note that
currently only "ShowTicket" is checked, extra right checks like
"ShowTicketComments" for "Comment" transactions are still done after
search, so the search count is still not 100% accurate.
diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index ac36e8538c..06462d3e88 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -97,7 +97,7 @@ sub _RoleGroupClass {
sub _RoleGroupsJoin {
my $self = shift;
- my %args = (New => 0, Class => '', Name => '', @_);
+ my %args = (New => 0, Class => '', Name => '', Alias => 'main', @_);
$args{'Class'} ||= $self->_RoleGroupClass;
@@ -118,7 +118,7 @@ sub _RoleGroupsJoin {
# Previously (before 4.4) this used an inner join.
my $groups = $self->Join(
TYPE => 'left',
- ALIAS1 => 'main',
+ ALIAS1 => $args{Alias},
FIELD1 => $instance,
TABLE2 => 'Groups',
FIELD2 => 'Instance',
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index a62b7de6a1..4ef4368bff 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -3000,7 +3000,8 @@ sub CurrentUserCanSee {
return unless @queues;
$self->Limit(
SUBCLAUSE => 'ACL',
- ALIAS => 'main',
+ # RT::Transactions::CurrentUserCanSee reuses RT::Tickets::CurrentUserCanSee
+ ALIAS => $self->isa('RT::Transactions') ? $self->_JoinTickets : 'main',
FIELD => 'Queue',
OPERATOR => 'IN',
VALUE => [ @queues ],
diff --git a/lib/RT/Transactions.pm b/lib/RT/Transactions.pm
index 21ca3cd86e..e03d2995a8 100644
--- a/lib/RT/Transactions.pm
+++ b/lib/RT/Transactions.pm
@@ -142,7 +142,28 @@ sub AddRecord {
my $self = shift;
my ($record) = @_;
- return unless $record->CurrentUserCanSee;
+ if ( $self->{_is_ticket_only_search} && RT->Config->Get('UseSQLForACLChecks') ) {
+ # UseSQLForACLChecks implies ShowTicket only, need to check out extra rights here.
+ my $type = $record->__Value('Type');
+ if ( $type eq 'Comment' ) {
+ return unless $record->CurrentUserHasRight('ShowTicketComments');
+ }
+ elsif ( $type eq 'CommentEmailRecord' ) {
+ return
+ unless $record->CurrentUserHasRight('ShowTicketComments')
+ && $record->CurrentUserHasRight('ShowOutgoingEmail');
+ }
+ elsif ( $type eq 'EmailRecord' ) {
+ return unless $record->CurrentUserHasRight('ShowOutgoingEmail');
+ }
+ elsif ( $type eq 'CustomField' ) {
+ return unless $record->CurrentUserCanSee;
+ }
+ }
+ else {
+ return unless $record->CurrentUserCanSee;
+ }
+
return $self->SUPER::AddRecord($record);
}
@@ -1111,6 +1132,28 @@ sub _parser {
return $self->_CloseParen unless $node->isLeaf;
}
);
+
+ # Determine if it's a ticket transaction search
+ $tree->traverse(
+ sub {
+ my $node = shift;
+ return unless $node->isLeaf and $node->getNodeValue;
+ my ($key, $subkey, $meta, $op, $value, $bundle)
+ = @{$node->getNodeValue}{qw/Key Subkey Meta Op Value Bundle/};
+ return unless $key eq 'ObjectType' && $value eq 'RT::Ticket' && $op eq '=';
+
+ my $is_ticket_only_search = 1;
+ while ( my $parent = $node->getParent ) {
+ last if $parent->isRoot;
+ if ( lc( $parent->getNodeValue // '' ) eq 'or' ) {
+ $is_ticket_only_search = 0;
+ last;
+ }
+ $node = $parent;
+ }
+ $self->{_is_ticket_only_search} ||= $is_ticket_only_search;
+ }
+ );
}
sub FromSQL {
@@ -1166,6 +1209,59 @@ sub Query {
return $self->{_sql_query};
}
+our $AUTOLOAD;
+sub AUTOLOAD {
+ my $self = shift;
+ my ($method) = ( $AUTOLOAD =~ /::(\w+)$/ );
+
+ no strict 'refs';
+
+ # Reuse RT::Tickets methods for UseSQLForACLChecks related joins/limitations.
+ if ( $self->{_is_ticket_only_search} && RT::Tickets->can($method) ) {
+ my @args = @_;
+ if ( $method eq '_RoleGroupsJoin' ) {
+ push @args, Alias => $self->_JoinTickets;
+ }
+
+ if ( $method eq '_RoleGroupClass' ) {
+ # We want ticket's role group class here
+ unshift @args, 'RT::Tickets';
+ }
+ else {
+ unshift @args, $self;
+ }
+
+ return "RT::Tickets::$method"->(@args);
+ }
+ elsif ( $method ne 'DESTROY' ) {
+ require Carp;
+ Carp::croak "Undefined subroutine &$AUTOLOAD called";
+ }
+}
+
+sub _DoSearch {
+ my $self = shift;
+ $self->CurrentUserCanSee if $self->{_is_ticket_only_search} && RT->Config->Get('UseSQLForACLChecks');
+ return $self->SUPER::_DoSearch( @_ );
+}
+
+sub _DoCount {
+ my $self = shift;
+ $self->CurrentUserCanSee if $self->{_is_ticket_only_search} && RT->Config->Get('UseSQLForACLChecks');
+ return $self->SUPER::_DoCount( @_ );
+}
+
+sub CleanSlate {
+ my $self = shift;
+ if ( $self->{_is_ticket_only_search} && RT->Config->Get('UseSQLForACLChecks') ) {
+ RT::Tickets::CleanSlate( $self, @_ ) ;
+ }
+ else {
+ $self->SUPER::CleanSlate(@_);
+ }
+ delete $self->{_is_ticket_only_search};
+}
+
RT::Base->_ImportOverlays();
1;
commit 0dc3835ffc44e3d34ea14089f76959ca23ae08dc
Merge: 4dbb8b0797 ef958dadf4
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 15:30:27 2023 -0400
Merge branch 'security/5.0/suppress-mailgate-REST-responses' into security/5.0.5-releng
commit 4dbb8b0797bdc693a95ba95e3e638113f65b63f4
Merge: af8015b825 f5bfef5be8
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 15:30:16 2023 -0400
Merge branch 'security/5.0/document-mailgate-apache-config' into security/5.0.5-releng
commit af8015b825be60c4c4f733f7451b753f0240ac21
Merge: 1280a7a987 4cb028365b
Author: sunnavy <sunnavy at bestpractical.com>
Date: Mon Sep 25 15:30:06 2023 -0400
Merge branch 'security/5.0/sanitize-sensitive-headers' into security/5.0.5-releng
commit ef958dadf4387f11817b1125ecd5cbfa973e4d66
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Aug 25 13:32:08 2023 -0400
Return mail processing details only in DevelMode
Don't return any system details in responses to POSTs
when not in DevelMode to avoid disclosing any information.
diff --git a/share/html/REST/1.0/NoAuth/mail-gateway b/share/html/REST/1.0/NoAuth/mail-gateway
index 9adad505d4..95ffe17137 100644
--- a/share/html/REST/1.0/NoAuth/mail-gateway
+++ b/share/html/REST/1.0/NoAuth/mail-gateway
@@ -59,9 +59,18 @@ use RT::Interface::Email;
$r->content_type('text/plain; charset=utf-8');
$m->error_format('text');
my ( $status, $error, $Ticket ) = RT::Interface::Email::Gateway( \%ARGS );
+
+# Obscure the message to avoid any information disclosure unless
+# in DevelMode.
+my $log_error;
+unless ( RT->Config->Get('DevelMode') ) {
+ $log_error = $error;
+ $error = 'operation unsuccessful';
+}
+
if ( $status == 1 ) {
$m->out("ok\n");
- if ( $Ticket && $Ticket->Id ) {
+ if ( $Ticket && $Ticket->Id && RT->Config->Get('DevelMode') ) {
$m->out( 'Ticket: ' . ($Ticket->Id || '') . "\n" );
$m->out( 'Queue: ' . ($Ticket->QueueObj->Name || '') . "\n" );
$m->out( 'Owner: ' . ($Ticket->OwnerObj->Name || '') . "\n" );
@@ -73,9 +82,11 @@ if ( $status == 1 ) {
}
else {
if ( $status == -75 ) {
+ RT->Logger->error("mail-gateway returned status -75: $log_error") if $log_error;
$m->out( "temporary failure - $error\n" );
}
else {
+ RT->Logger->error("mail-gateway error: $log_error") if $log_error;
$m->out( "not ok - $error\n" );
}
}
commit f5bfef5be8bf1ca60faa73caea9203d1d239da25
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Aug 25 13:57:24 2023 -0400
Document restricting access to REST 1.0 mail-gateway
diff --git a/docs/web_deployment.pod b/docs/web_deployment.pod
index 22c2ef30f5..5d2f3eccb6 100644
--- a/docs/web_deployment.pod
+++ b/docs/web_deployment.pod
@@ -154,6 +154,31 @@ RT to access the Authorization header.
More information is available in L<RT::Authen::Token>.
+=head3 Restricting the REST 1.0 mail-gateway
+
+RT processes email via a REST 1.0 endpoint. If you accept email on the same
+server as your running RT, you can restrict this endpoint to localhost only
+with a configuration like the following:
+
+ # Accept requests only from localhost
+ <Location /REST/1.0/NoAuth/mail-gateway>
+ Require local
+ </Location>
+
+If you run C<bin/rt-mailgate> on a separate server, you can update
+the above to allow additional IP addresses.
+
+ <Location /REST/1.0/NoAuth/mail-gateway>
+ Require ip 127.0.0.1 ::1 192.0.2.0 # Add you actual IPs
+ </Location>
+
+See the L<Apache documentation|https://httpd.apache.org/docs/2.4/mod/mod_authz_host.html>
+for additional configuration options.
+
+After adding this configuration, test receiving email and confirm
+your C<bin/rt-mailgate> utility and C</etc/aliases> configurations
+can successfully submit email to RT.
+
=head2 nginx
C<nginx> requires that you start RT's fastcgi process externally, for
commit 4cb028365b4ef7524bd1f1192b5c0b5f1f97d66a
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Sep 12 16:21:26 2023 -0400
Sanitize non-crypt headers used in RT internally from incoming email
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index e034e13daa..c98223749a 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -161,6 +161,10 @@ sub Gateway {
);
}
+ # Clean up sensitive headers. Crypt related headers are cleaned up in RT::Interface::Email::Crypt::VerifyDecrypt
+ my @headers = qw( RT-Attach RT-Send-Cc RT-Send-Bcc RT-Message-ID RT-DetectedAutoGenerated RT-Squelch-Replies-To );
+ $Message->head->delete($_) for @headers;
+
#Set up a queue object
my $SystemQueueObj = RT::Queue->new( RT->SystemUser );
$SystemQueueObj->Load( $args{'queue'} );
commit 2b9ff1a8218bdbf4931fcea8c3676370669e1b24
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Wed Nov 4 08:45:44 2020 -0500
Clear all RT crypt headers from incoming email before processing
The old code did not delete the X-RT-SMIME-Status or X-RT-GnuPG-Status
headers by accident because of missing parens.
diff --git a/lib/RT/Interface/Email/Crypt.pm b/lib/RT/Interface/Email/Crypt.pm
index bc3427ca49..9d4d4fe584 100644
--- a/lib/RT/Interface/Email/Crypt.pm
+++ b/lib/RT/Interface/Email/Crypt.pm
@@ -73,13 +73,14 @@ sub VerifyDecrypt {
);
# we clean all possible headers
- my @headers =
+ my @headers = (
qw(
X-RT-Incoming-Encryption
X-RT-Incoming-Signature X-RT-Privacy
X-RT-Sign X-RT-Encrypt
),
- map "X-RT-$_-Status", RT::Crypt->Protocols;
+ map "X-RT-$_-Status", RT::Crypt->Protocols
+ );
foreach my $p ( $args{'Message'}->parts_DFS ) {
$p->head->delete($_) for @headers;
}
-----------------------------------------------------------------------
Summary of changes:
docs/web_deployment.pod | 25 +++++++
lib/RT/Articles.pm | 5 ++
lib/RT/Assets.pm | 5 ++
lib/RT/Attachment.pm | 11 +++
lib/RT/Catalog.pm | 22 ++++++
lib/RT/Catalogs.pm | 5 ++
lib/RT/Class.pm | 33 +++++++++
lib/RT/Classes.pm | 5 ++
lib/RT/CustomField.pm | 22 ++++++
lib/RT/CustomFields.pm | 5 ++
lib/RT/CustomRole.pm | 22 ++++++
lib/RT/CustomRoles.pm | 6 ++
lib/RT/Group.pm | 34 ++++++++-
lib/RT/Groups.pm | 5 ++
lib/RT/Interface/Email.pm | 4 +
lib/RT/Interface/Email/Crypt.pm | 5 +-
lib/RT/Interface/Web.pm | 22 ++++++
lib/RT/ObjectCustomFieldValue.pm | 1 +
lib/RT/Queue.pm | 23 ++++++
lib/RT/Queues.pm | 5 ++
lib/RT/REST2/Resource/Article.pm | 10 +--
lib/RT/REST2/Resource/Asset.pm | 6 +-
lib/RT/REST2/Resource/Collection.pm | 28 +++----
lib/RT/REST2/Resource/CustomField.pm | 15 ----
lib/RT/REST2/Resource/Group.pm | 39 +++-------
lib/RT/REST2/Resource/GroupMembers.pm | 2 -
lib/RT/REST2/Resource/ObjectCustomFieldValue.pm | 6 --
lib/RT/REST2/Resource/RT.pm | 4 +-
lib/RT/REST2/Resource/Record.pm | 17 ++++-
lib/RT/REST2/Resource/Ticket.pm | 11 +--
lib/RT/REST2/Resource/User.pm | 3 +-
lib/RT/SearchBuilder.pm | 5 ++
lib/RT/SearchBuilder/Role/Roles.pm | 4 +-
lib/RT/Ticket.pm | 4 +
lib/RT/Tickets.pm | 11 ++-
lib/RT/Transactions.pm | 98 ++++++++++++++++++++++++-
lib/RT/Users.pm | 5 ++
share/html/Elements/CollectionList | 6 +-
share/html/REST/1.0/NoAuth/mail-gateway | 13 +++-
share/html/Search/Results.html | 10 +--
share/html/Search/Results.tsv | 10 +--
t/mail/gateway.t | 2 +-
t/mail/han-encodings.t | 2 +-
t/mail/sendmail-plaintext.t | 2 +-
t/mail/sendmail.t | 2 +-
t/rest2/articles.t | 2 +-
t/rest2/assets.t | 2 +-
t/rest2/attachments.t | 4 +-
t/rest2/cf-image.t | 2 +-
t/rest2/customfields.t | 2 +-
t/rest2/group-members.t | 24 ++----
t/rest2/searches.t | 2 +-
t/rest2/tickets.t | 4 +-
t/rest2/transactions.t | 4 +-
t/ticket/interface.t | 2 +-
55 files changed, 471 insertions(+), 157 deletions(-)
hooks/post-receive
--
rt
More information about the rt-commit
mailing list