From edbc7371a52fc5d0032e934d2456b5f39da317f1 Mon Sep 17 00:00:00 2001 From: Milan Rakic Date: Thu, 9 May 2019 17:11:36 +0200 Subject: [PATCH] Fixed: External images are automatically loaded in forward screen (bug#14398). --- CHANGES.md | 2 + Kernel/Config/Files/Ticket.xml | 11 ++ Kernel/Modules/AgentTicketActionCommon.pm | 9 + Kernel/Modules/AgentTicketAttachment.pm | 25 ++- Kernel/Modules/AgentTicketCompose.pm | 9 + Kernel/Modules/AgentTicketForward.pm | 9 + Kernel/Modules/AgentTicketPhone.pm | 9 + Kernel/Modules/CustomerTicketAttachment.pm | 24 ++- Kernel/Output/HTML/Layout.pm | 10 +- scripts/test/Layout/RichTextDocumentServe.t | 8 +- .../test/Selenium/Agent/AgentTicketForward.t | 155 ++++++++++++------ 11 files changed, 201 insertions(+), 70 deletions(-) --- a/Kernel/Config/Files/Ticket.xml +++ b/Kernel/Config/Files/Ticket.xml @@ -1866,6 +1866,17 @@ + + Makes the application block external content loading. + Ticket + Frontend::Agent + + + + Shows the customer user information (phone and email) in the compose screen. Ticket --- a/Kernel/Modules/AgentTicketActionCommon.pm +++ b/Kernel/Modules/AgentTicketActionCommon.pm @@ -1316,6 +1316,15 @@ sub Run { # set Body var to calculated content $GetParam{Body} = $Body; + # Strip out external content if needed. + if ( $ConfigObject->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + my %SafetyCheckResult = $Kernel::OM->Get('Kernel::System::HTMLUtils')->Safety( + String => $GetParam{Body}, + NoExtSrcLoad => 1, + ); + $GetParam{Body} = $SafetyCheckResult{String}; + } + if ( $Self->{ReplyToArticle} ) { my $TicketSubjectRe = $ConfigObject->Get('Ticket::SubjectRe') || 'Re'; $GetParam{Subject} = $TicketSubjectRe . ': ' . $Self->{ReplyToArticleContent}{Subject}; --- a/Kernel/Modules/AgentTicketAttachment.pm +++ b/Kernel/Modules/AgentTicketAttachment.pm @@ -168,15 +168,6 @@ sub Run { # set filename for inline viewing $Data{Filename} = "Ticket-$Article{TicketNumber}-ArticleID-$Article{ArticleID}.html"; - my $LoadExternalImages = $ParamObject->GetParam( - Param => 'LoadExternalImages' - ) || 0; - - # safety check only on customer article - if ( !$LoadExternalImages && $Article{SenderType} ne 'customer' ) { - $LoadExternalImages = 1; - } - # generate base url my $URL = 'Action=AgentTicketAttachment;Subaction=HTMLView' . ";ArticleID=$ArticleID;FileID="; @@ -187,6 +178,22 @@ sub Run { UserID => $Self->{UserID}, ); + # Do not load external images if 'BlockLoadingRemoteContent' is enabled. + my $LoadExternalImages; + if ( $ConfigObject->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + $LoadExternalImages = 0; + } + else { + $LoadExternalImages = $ParamObject->GetParam( + Param => 'LoadExternalImages' + ) || 0; + + # Safety check only on customer article. + if ( !$LoadExternalImages && $Article{SenderType} ne 'customer' ) { + $LoadExternalImages = 1; + } + } + # reformat rich text document to have correct charset and links to # inline documents %Data = $LayoutObject->RichTextDocumentServe( --- a/Kernel/Modules/AgentTicketCompose.pm +++ b/Kernel/Modules/AgentTicketCompose.pm @@ -1170,6 +1170,15 @@ sub Run { UploadCacheObject => $UploadCacheObject, ); + # Strip out external content if needed. + if ( $ConfigObject->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + my %SafetyCheckResult = $Kernel::OM->Get('Kernel::System::HTMLUtils')->Safety( + String => $Data{Body}, + NoExtSrcLoad => 1, + ); + $Data{Body} = $SafetyCheckResult{String}; + } + # restrict number of body lines if configured if ( $Data{Body} --- a/Kernel/Modules/AgentTicketForward.pm +++ b/Kernel/Modules/AgentTicketForward.pm @@ -286,6 +286,15 @@ sub Form { AttachmentsInclude => 1, ); + # Strip out external content if needed. + if ( $ConfigObject->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + my %SafetyCheckResult = $Kernel::OM->Get('Kernel::System::HTMLUtils')->Safety( + String => $Data{Body}, + NoExtSrcLoad => 1, + ); + $Data{Body} = $SafetyCheckResult{String}; + } + if ( $LayoutObject->{BrowserRichText} ) { # prepare body, subject, ReplyTo ... --- a/Kernel/Modules/AgentTicketPhone.pm +++ b/Kernel/Modules/AgentTicketPhone.pm @@ -351,6 +351,15 @@ sub Run { $Article{ContentType} = 'text/plain'; } + # Strip out external content if needed. + if ( $ConfigObject->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + my %SafetyCheckResult = $Kernel::OM->Get('Kernel::System::HTMLUtils')->Safety( + String => $Article{Body}, + NoExtSrcLoad => 1, + ); + $Article{Body} = $SafetyCheckResult{String}; + } + # show customer info if ( $ConfigObject->Get('Ticket::Frontend::CustomerInfoCompose') ) { if ( $Article{CustomerUserID} ) { --- a/Kernel/Modules/CustomerTicketAttachment.pm +++ b/Kernel/Modules/CustomerTicketAttachment.pm @@ -121,14 +121,6 @@ sub Run { # unset filename for inline viewing $Data{Filename} = "Ticket-$Article{TicketNumber}-ArticleID-$Article{ArticleID}.html"; - # safety check only on customer article - my $LoadExternalImages = $ParamObject->GetParam( - Param => 'LoadExternalImages' - ) || 0; - if ( !$LoadExternalImages && $Article{SenderType} ne 'customer' ) { - $LoadExternalImages = 1; - } - # generate base url my $URL = 'Action=CustomerTicketAttachment;Subaction=HTMLView' . ";ArticleID=$ArticleID;FileID="; @@ -139,6 +131,22 @@ sub Run { UserID => $Self->{UserID}, ); + # Do not load external images if 'BlockLoadingRemoteContent' is enabled. + my $LoadExternalImages; + if ( $Kernel::OM->Get('Kernel::Config')->Get('Ticket::Frontend::BlockLoadingRemoteContent') ) { + $LoadExternalImages = 0; + } + else { + $LoadExternalImages = $ParamObject->GetParam( + Param => 'LoadExternalImages' + ) || 0; + + # Safety check only on customer article. + if ( !$LoadExternalImages && $Article{SenderType} ne 'customer' ) { + $LoadExternalImages = 1; + } + } + # reformat rich text document to have correct charset and links to # inline documents %Data = $LayoutObject->RichTextDocumentServe( --- a/Kernel/Output/HTML/Layout.pm +++ b/Kernel/Output/HTML/Layout.pm @@ -4389,8 +4389,7 @@ sub RichTextDocumentServe { if ( !$Param{LoadExternalImages} ) { - # Strip out external images, but show a confirmation button to - # load them explicitly. + # Strip out external content. my %SafetyCheckResult = $Kernel::OM->Get('Kernel::System::HTMLUtils')->Safety( String => $Param{Data}->{Content}, NoApplet => 1, @@ -4405,7 +4404,12 @@ sub RichTextDocumentServe { $Param{Data}->{Content} = $SafetyCheckResult{String}; - if ( $SafetyCheckResult{Replace} ) { + # Show confirmation button to load external content explicitly only if BlockLoadingRemoteContent is disabled. + if ( + $SafetyCheckResult{Replace} + && !$Kernel::OM->Get('Kernel::Config')->Get('Ticket::Frontend::BlockLoadingRemoteContent') + ) + { # Generate blocker message. my $Message = $Self->Output( TemplateFile => 'AttachmentBlocker' ); --- a/scripts/test/Layout/RichTextDocumentServe.t +++ b/scripts/test/Layout/RichTextDocumentServe.t @@ -14,7 +14,6 @@ use vars (qw($Self)); local $ENV{SCRIPT_NAME} = 'index.pl'; -# get needed objects $Kernel::OM->ObjectParamAdd( 'Kernel::System::UnitTest::Helper' => { RestoreDatabase => 1, @@ -30,6 +29,13 @@ $Kernel::OM->ObjectParamAdd( ); my $LayoutObject = $Kernel::OM->Get('Kernel::Output::HTML::Layout'); +# Disable global external content blocking. +$Helper->ConfigSettingChange( + Valid => 1, + Key => 'Ticket::Frontend::BlockLoadingRemoteContent', + Value => 0, +); + my @Tests = ( { Name => '', --- a/scripts/test/Selenium/Agent/AgentTicketForward.t +++ b/scripts/test/Selenium/Agent/AgentTicketForward.t @@ -12,30 +12,29 @@ use utf8; use vars (qw($Self)); -# get selenium object my $Selenium = $Kernel::OM->Get('Kernel::System::UnitTest::Selenium'); $Selenium->RunTest( sub { - # get helper object my $Helper = $Kernel::OM->Get('Kernel::System::UnitTest::Helper'); my $ConfigObject = $Kernel::OM->Get('Kernel::Config'); + my $TicketObject = $Kernel::OM->Get('Kernel::System::Ticket'); - # disable check email addresses + # Disable check email addresses. $Helper->ConfigSettingChange( Key => 'CheckEmailAddresses', Value => 0, ); - # do not check RichText + # Do not check RichText. $Helper->ConfigSettingChange( Valid => 1, Key => 'Frontend::RichText', Value => 0 ); - # do not check service and type + # Do not check service and type. $Helper->ConfigSettingChange( Valid => 1, Key => 'Ticket::Service', @@ -47,10 +46,16 @@ $Selenium->RunTest( Value => 0 ); - # get ticket object - my $TicketObject = $Kernel::OM->Get('Kernel::System::Ticket'); + # Disable global external content blocking. + $Helper->ConfigSettingChange( + Valid => 1, + Key => 'Ticket::Frontend::BlockLoadingRemoteContent', + Value => 0, + ); + + my $RandomID = $Helper->GetRandomID(); - # create test ticket + # Create test ticket. my $TicketID = $TicketObject->TicketCreate( Title => 'Selenium ticket', Queue => 'Raw', @@ -67,7 +72,7 @@ $Selenium->RunTest( "TicketCreate - ID $TicketID", ); - # create test email article + # Create test email article. my $ArticleID = $TicketObject->ArticleCreate( TicketID => $TicketID, ArticleType => 'email-external', @@ -85,23 +90,17 @@ $Selenium->RunTest( "ArticleCreate - ID $ArticleID", ); - # create test user and login + # Create test user. my $TestUserLogin = $Helper->TestUserCreate( Groups => [ 'admin', 'users' ], ) || die "Did not get test user"; - $Selenium->Login( - Type => 'Agent', - User => $TestUserLogin, - Password => $TestUserLogin, - ); - - # get test user ID + # Get test user ID. my $TestUserID = $Kernel::OM->Get('Kernel::System::User')->UserLookup( UserLogin => $TestUserLogin, ); - # add test customer for testing + # Add test customer for testing. my $TestCustomer = 'Customer' . $Helper->GetRandomID(); my $TestCustomerUserID = $Kernel::OM->Get('Kernel::System::CustomerUser')->CustomerUserAdd( Source => 'CustomerUser', @@ -181,7 +180,7 @@ $Selenium->RunTest( "Ticket with ticket ID $TicketID is deleted" ); - # delete created test customer user + # Delete created test customer user. my $DBObject = $Kernel::OM->Get('Kernel::System::DB'); $TestCustomer = $DBObject->Quote($TestCustomer); $Success = $DBObject->Do( @@ -193,12 +192,14 @@ $Selenium->RunTest( "Delete customer user - $TestCustomer", ); - # make sure the cache is correct + my $CacheObject = $Kernel::OM->Get('Kernel::System::Cache'); + + # Make sure the cache is correct. for my $Cache ( - qw (Ticket CustomerUser ) + qw (Ticket CustomerUser) ) { - $Kernel::OM->Get('Kernel::System::Cache')->CleanUp( + $CacheObject->CleanUp( Type => $Cache, ); }