Attachment 123107 Details for Bug 25464 – Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

[patch] Bug 25464 - Add ability to specify client IP and SIP account used in SIP2 logging

Bug-25464---Add-ability-to-specify-client-IP-and-S.patch (text/plain), 6.39 KB, created by Nick Clemens (kidclamp) on 2021-07-23 12:24:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-07-23 12:24:56 UTC

Size: 6.39 KB

patch

obsolete

>From 2d63c5a0457345f5ab443ab9e27fc8b4ed9849a9 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> 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 9156a952f3..7152d78a13 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 4db8f89be8..1e749b0014 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 940c2d0397..424568336e 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 a2a0cc58b8..e532043250 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.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 25464: 104759 | 104794 | 104795 | 119706 | 123107 | 123182 | 123183