From e9bc90ebb0dfa5c9724e7473b4003275c0313d7f Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 18 Oct 2021 12:28:27 +0000 Subject: [PATCH] Bug 29264: SIP config allows use of non-branchcode institution ids causes workers to die without responding MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If is entirely possible to create an SIP institution whose ID does not match a valid branchcode in Koha's SIP config. In fact, Koha's example SIP config contains an example of this ( kohalibrary / kohalibrary2 ). If a SIP login uses an institution with an id that doesn't match a valid branchcode, everything will appear to work, but the SIP worker will die anywhere that Koha gets the branch from the userenv and assumes it is valid. The repercussions of this are that actions such as the checkout message simply die and do not return a response message to the requestor. At the very least, we should output a warning to the SIP log. I think we should strongly consider disallowing institution ids in the SIP config that do not match valid branchcodes. In this scenario, attempting to start the SIP server should result in a error message with the SIP server exiting immediately. Test Plan: 1) Apply this patch 2) Make a sip login that uses an instution whose id is *not* a valid branchcode 3) Start the SIP server 4) Check sip.log, you should see a warning similar to the following: [2021/10/18 12:18:29] [2068079] [ERROR] ERROR: Institution kohalibrary does does not match a branchcode. This can cause unexpected behavior. C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) Signed-off-by: David Nind Signed-off-by: Joonas Kylmälä Signed-off-by: Jonathan Druart --- C4/SIP/SIPServer.pm | 3 +++ C4/SIP/Sip/Configuration.pm | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/C4/SIP/SIPServer.pm b/C4/SIP/SIPServer.pm index b4cf77f539..995d448d84 100644 --- a/C4/SIP/SIPServer.pm +++ b/C4/SIP/SIPServer.pm @@ -30,6 +30,9 @@ use base qw(Net::Server::PreFork); use constant LOG_SIP => "local6"; # Local alias for the logging facility + +set_logger( Koha::Logger->get( { interface => 'sip' } ) ); + # # Main # not really, since package SIPServer # diff --git a/C4/SIP/Sip/Configuration.pm b/C4/SIP/Sip/Configuration.pm index 3f5bdaaa4b..006d6b5cf3 100644 --- a/C4/SIP/Sip/Configuration.pm +++ b/C4/SIP/Sip/Configuration.pm @@ -9,8 +9,10 @@ package C4::SIP::Sip::Configuration; use strict; use warnings; use XML::Simple qw(:strict); +use List::Util qw(uniq); use C4::SIP::Sip qw(siplog); +use Koha::Libraries; my $parser = XML::Simple->new( KeyAttr => { @@ -47,6 +49,12 @@ sub new { } $cfg->{listeners} = \%listeners; + my @branchcodes = Koha::Libraries->search()->get_column('branchcode'); + my @institutions = uniq( keys %{ $cfg->{institutions} } ); + foreach my $i ( @institutions ) { + siplog("LOG_ERR", "ERROR: Institution $i does does not match a branchcode. This can cause unexpected behavior.") unless grep( /^$i$/, @branchcodes ); + } + return bless $cfg, $class; } -- 2.39.5