From 006c70f4f2112f7057e5c6228bfd4dd6684cf976 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 12 May 2020 07:11:12 -0400 Subject: [PATCH] Bug 25464: Add ability to specify client IP and SIP account used in SIP2 logging Bug 15253 originally had the ability to specify the incoming IP address used for a given log statement via SIP, as well as the SIP2 account that was in use at the time. This data is very helpful for debugging purposes, and should be brought back. Test Plan: 1) Apply this patch 2) Update you SIP ConversionPattern to "[%d] [%p] %X{accountid}@%X{peeraddr}: %m %l %n" 3) Restart SIP 4) Use the SIP cli tester to make some SIP requests 5) View the SIP2 log, note the account id and client ip address show in the log! Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart --- C4/SIP/SIPServer.pm | 7 +++++ Koha/Logger.pm | 43 ++++++++++++++++++++++++++ debian/koha-common.postinst | 2 +- debian/templates/log4perl-site.conf.in | 2 +- etc/log4perl.conf | 2 +- t/Logger.t | 15 +++++++-- 6 files changed, 66 insertions(+), 5 deletions(-) diff --git a/C4/SIP/SIPServer.pm b/C4/SIP/SIPServer.pm index 4f0a0a627f..df52d5c9af 100644 --- a/C4/SIP/SIPServer.pm +++ b/C4/SIP/SIPServer.pm @@ -106,6 +106,9 @@ sub process_request { $self->{account} = undef; # Clear out the account from the last request, it may be different $self->{logger} = set_logger( Koha::Logger->get( { interface => 'sip' } ) ); + # Flush previous MDCs to prevent accidentally leaking incorrect MDC-entries + Koha::Logger->clear_mdc(); + my $sockname = getsockname(STDIN); # Check if socket connection is IPv6 before resolving address @@ -180,6 +183,10 @@ sub raw_transport { ) ); + # Set MDCs after properly authenticating + Koha::Logger->put_mdc( "accountid", $self->{account}->{id} ); + Koha::Logger->put_mdc( "peeraddr", $self->{server}->{peeraddr} ); + siplog("LOG_DEBUG", "raw_transport: uname/inst: '%s/%s'", $self->{account}->{id}, $self->{account}->{institution}); diff --git a/Koha/Logger.pm b/Koha/Logger.pm index c91a3bb3b4..f91e5a2277 100644 --- a/Koha/Logger.pm +++ b/Koha/Logger.pm @@ -70,6 +70,49 @@ sub get { return $self; } +=head2 put_mdc + +my $foo = $logger->put_mdc('foo', $foo ); + +put_mdc sets global thread specific data that can be access later when generating log lines +via the "%X{key}" placeholder in Log::Log4perl::Layout::PatternLayouts. + +=cut + +sub put_mdc { + my ( $self, $key, $value ) = @_; + + Log::Log4perl::MDC->put( $key, $value ); +} + +=head2 get_mdc + +my $foo = $logger->get_mdc('foo'); + +Retrieves the stored mdc value from the stored map. + +=cut + +sub get_mdc { + my ( $self, $key ) = @_; + + return Log::Log4perl::MDC->get( $key ); +} + +=head2 clear_mdc + +$logger->clear_mdc(); + +Removes *all* stored key/value pairs from the MDC map. + +=cut + +sub clear_mdc { + my ( $self, $key ) = @_; + + return Log::Log4perl::MDC->remove( $key ); +} + =head1 INTERNALS =head2 AUTOLOAD diff --git a/debian/koha-common.postinst b/debian/koha-common.postinst index 06fcabb0bd..f8914fb25d 100644 --- a/debian/koha-common.postinst +++ b/debian/koha-common.postinst @@ -182,7 +182,7 @@ log4perl.appender.SIP=Log::Log4perl::Appender::File log4perl.appender.SIP.filename=/var/log/koha/$site/sip.log log4perl.appender.SIP.mode=append log4perl.appender.SIP.layout=PatternLayout -log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %m %l%n +log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %X{accountid}@%X{peeraddr}: %m %l%n log4perl.appender.SIP.utf8=1 EOF diff --git a/debian/templates/log4perl-site.conf.in b/debian/templates/log4perl-site.conf.in index b0726dce09..a95ab73163 100644 --- a/debian/templates/log4perl-site.conf.in +++ b/debian/templates/log4perl-site.conf.in @@ -35,7 +35,7 @@ log4perl.appender.SIP=Log::Log4perl::Appender::File log4perl.appender.SIP.filename=__LOG_DIR__/sip.log log4perl.appender.SIP.mode=append log4perl.appender.SIP.layout=PatternLayout -log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %m %l%n +log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %X{accountid}@%X{peeraddr}: %m %l%n log4perl.appender.SIP.utf8=1 diff --git a/etc/log4perl.conf b/etc/log4perl.conf index 7dbc950282..6736c3d1ba 100644 --- a/etc/log4perl.conf +++ b/etc/log4perl.conf @@ -35,7 +35,7 @@ log4perl.appender.SIP=Log::Log4perl::Appender::File log4perl.appender.SIP.filename=__LOG_DIR__/sip.log log4perl.appender.SIP.mode=append log4perl.appender.SIP.layout=PatternLayout -log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %m %l%n +log4perl.appender.SIP.layout.ConversionPattern=[%d] [%P] [%p] %X{accountid}@%X{peeraddr}: %m %l%n log4perl.appender.SIP.utf8=1 log4perl.logger.plack-opac = WARN, PLACKOPAC diff --git a/t/Logger.t b/t/Logger.t index 9aa6819b30..870ecd9029 100755 --- a/t/Logger.t +++ b/t/Logger.t @@ -26,7 +26,7 @@ use Test::More tests => 1; use Test::Warn; subtest 'Test01 -- Simple tests for Koha::Logger' => sub { - plan tests => 6; + plan tests => 10; my $ret; t::lib::Mocks::mock_config('log4perl_conf', undef); @@ -52,7 +52,7 @@ HERE ok( $@, 'Logger did not init correctly without permission'); system("chmod 700 $log"); - my $logger= Koha::Logger->get({ interface => 'intranet' }); + my $logger = Koha::Logger->get( { interface => 'intranet' } ); is( exists $logger->{logger}, 1, 'Log4perl config found'); is( $logger->warn('Message 1'), 1, '->warn returned a value' ); warning_is { $ret = $logger->catastrophe } @@ -60,6 +60,17 @@ HERE "Undefined method raises warning"; is( $ret, undef, "'catastrophe' method undefined"); + Koha::Logger->put_mdc( 'foo', 'bar' ); + is( Koha::Logger->get_mdc( 'foo' ), 'bar', "MDC value via put_mdc is correct" ); + + Koha::Logger->put_mdc( 'foo', undef ); + is( Koha::Logger->get_mdc( 'foo' ), undef, "Updated MDC value to undefined via put_mdc is correct" ); + + Koha::Logger->put_mdc( 'foo', 'baz' ); + is( Koha::Logger->get_mdc( 'foo' ), 'baz', "Updated MDC value via put_mdc is correct" ); + + Koha::Logger->clear_mdc(); + is( Koha::Logger->get_mdc( 'foo' ), undef, "MDC value was cleared by clear_mdc" ); }; sub mytempfile { -- 2.39.5