From 33fd89a0f9c090d73128ddc4c75ec8e102171ea1 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 6 Jun 2023 12:16:22 -0400 Subject: [PATCH] Bug 33926: Add ability to specify fields allowed in a response Content-Type: text/plain; charset=utf-8 At this time, we can specify fields to hide in SIP response at the login level. From a security perspective, it would be useful to also be able to specify which fields are allowed in a response. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/Message.t Signed-off-by: Sam Lau Signed-off-by: Marcel de Rooy --- C4/SIP/Sip.pm | 30 +++++++++++++++++++++--------- etc/SIPconfig.xml | 3 ++- t/db_dependent/SIP/Message.t | 31 ++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm index 4d662a26b7..c29bff282f 100644 --- a/C4/SIP/Sip.pm +++ b/C4/SIP/Sip.pm @@ -11,7 +11,7 @@ use Encode; use POSIX qw(strftime); use Socket qw(:crlf); use IO::Handle; -use List::Util qw(first); +use List::Util qw(any); use C4::SIP::Sip::Constants qw(SIP_DATETIME FID_SCREEN_MSG); use C4::SIP::Sip::Checksum qw(checksum); @@ -60,10 +60,7 @@ sub timestamp { sub add_field { my ($field_id, $value, $server) = @_; - if ( my $hide_fields = $server->{account}->{hide_fields} ) { - my @fields = split( ',', $hide_fields ); - return q{} if first { $_ eq $field_id } @fields; - } + return q{} if should_hide( $field_id, $value, $server ); my ($i, $ent); @@ -94,10 +91,7 @@ sub add_field { sub maybe_add { my ($fid, $value, $server) = @_; - if ( my $hide_fields = $server->{account}->{hide_fields} ) { - my @fields = split( ',', $hide_fields ); - return q{} if first { $_ eq $fid } @fields; - } + return q{} if should_hide( $fid, $value, $server ); if ( $fid eq FID_SCREEN_MSG && $server->{account}->{screen_msg_regex} && defined($value)) { foreach my $regex ( @@ -114,6 +108,24 @@ sub maybe_add { : ''; } +sub should_hide { + my ( $field_id, $value, $server ) = @_; + + my $allow_fields = $server->{account}->{allow_fields}; + if ($allow_fields) { + my @fields = split( ',', $allow_fields ); + return 1 unless any { $_ eq $field_id } @fields; + } + + my $hide_fields = $server->{account}->{hide_fields}; + if ($hide_fields) { + my @fields = split( ',', $hide_fields ); + return 1 if any { $_ eq $field_id } @fields; + } + + return 0; +} + # # add_count() produce fixed four-character count field, # or a string of four spaces if the count is invalid for some diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml index 5264c2674c..6f131f4ce8 100644 --- a/etc/SIPconfig.xml +++ b/etc/SIPconfig.xml @@ -56,7 +56,8 @@ - + + sub { subtest 'Testing Patron Info Request V2' => sub { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; - plan tests => 24; + plan tests => 32; $C4::SIP::Sip::protocol_version = 2; test_request_patron_info_v2(); $schema->storage->txn_rollback; @@ -706,15 +706,40 @@ sub test_request_patron_info_v2 { $respcode = substr( $response, 0, 2 ); check_field( $respcode, $response, FID_PERSONAL_NAME, 'X' . $patron2->{surname} . 'Y', 'Check customized patron name' ); + # Test hide_fields undef $response; - $server->{account}->{hide_fields} = "BD,BE,BF,PB"; + $server->{account}->{hide_fields} = join( ",", FID_HOME_ADDR, FID_EMAIL, FID_HOME_PHONE, FID_PATRON_BIRTHDATE ); $msg->handle_patron_info( $server ); $respcode = substr( $response, 0, 2 ); check_field( $respcode, $response, FID_HOME_ADDR, undef, 'Home address successfully stripped from response' ); check_field( $respcode, $response, FID_EMAIL, undef, 'Email address successfully stripped from response' ); check_field( $respcode, $response, FID_HOME_PHONE, undef, 'Home phone successfully stripped from response' ); check_field( $respcode, $response, FID_PATRON_BIRTHDATE, undef, 'Date of birth successfully stripped from response' ); - $server->{account}->{hide_fields} = ""; + delete $server->{account}->{hide_fields}; + + # Test allow_fields + undef $response; + $server->{account}->{allow_fields} = join( ",", FID_EMAIL, FID_HOME_PHONE ); + $msg->handle_patron_info( $server ); + $respcode = substr( $response, 0, 2 ); + check_field( $respcode, $response, FID_HOME_ADDR, undef, 'Home address successfully stripped from response' ); + check_field( $respcode, $response, FID_EMAIL, $patron2->{email}, 'Email address successfully retained in response' ); + check_field( $respcode, $response, FID_HOME_PHONE, $patron2->{phone}, 'Home phone successfully retained in from response' ); + check_field( $respcode, $response, FID_PATRON_BIRTHDATE, undef, 'Date of birth successfully stripped from response' ); + delete $server->{account}->{allow_fields}; + + # Test hide_fields should take precedence over allow_fields + undef $response; + $server->{account}->{hide_fields} = join( ",", FID_HOME_ADDR, FID_EMAIL, FID_PATRON_BIRTHDATE ); + $server->{account}->{allow_fields} = join( ",", FID_EMAIL, FID_HOME_PHONE ); + $msg->handle_patron_info( $server ); + $respcode = substr( $response, 0, 2 ); + check_field( $respcode, $response, FID_HOME_ADDR, undef, 'Home address successfully stripped from response' ); + check_field( $respcode, $response, FID_EMAIL, undef, 'Email address successfully stripped from response' ); + check_field( $respcode, $response, FID_HOME_PHONE, $patron2->{phone}, 'Home phone successfully retained in from response' ); + check_field( $respcode, $response, FID_PATRON_BIRTHDATE, undef, 'Date of birth successfully stripped from response' ); + delete $server->{account}->{hide_fields}; + delete $server->{account}->{allow_fields}; # Check empty password and verify CQ again $siprequest = PATRON_INFO. 'engYYYYMMDDZZZZHHMMSS'.'Y '. -- 2.30.2