Attachment 127425 Details for Bug 29264 – Bug 29264: SIP config allows use of non-branchcode institution ids causes workers to die without responding

[patch] Bug 29264: SIP config allows use of non-branchcode institution ids causes workers to die without responding

Bug-29264-SIP-config-allows-use-of-non-branchcode-.patch (text/plain), 2.95 KB, created by David Nind on 2021-11-08 08:02:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2021-11-08 08:02:53 UTC

Size: 2.95 KB

patch

obsolete

>From 4b9a4d6d73bbc07135808c273db677758ccf476e Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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
>
>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 <david@davidnind.com>
>---
> 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.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 29264: 126432 | 127425 | 127606