Bug 33926: Add ability to specify fields allowed in a response

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 <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Kyle Hall 2023-06-06 12:16:22 -04:00 committed by Tomas Cohen Arazi
parent 06c2b4898b
commit 120b991529
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
3 changed files with 51 additions and 13 deletions

View file

@ -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

View file

@ -56,7 +56,8 @@
<login id="staff" password="staff" delimiter="|" error-detect="enabled" institution="CPL" encoding="ascii" checked_in_ok="1" payment_type_writeoff="06" disallow_overpayment="1" />
<login id="koha" password="koha" delimiter="|" error-detect="enabled" institution="kohalibrary" encoding="utf8" />
<login id="koha2" password="koha" institution="kohalibrary2" terminator="CR" />
<login id="lpl-sc" password="1234" institution="LPL" />
<login id="lpl-sc" password="1234" institution="LPL" allow_fields="AO,AA,AE"/>
<!-- allow_fields hides all fields not in the list, it is the inverse of hide_fields ( hide_fields takes precedence ) -->
<login id="lpl-sc-beacock" password="xyzzy"
delimiter="|" error-detect="enabled" institution="LPL"
send_patron_home_library_in_af="1"

View file

@ -63,7 +63,7 @@ subtest 'Testing Patron Status Request V2' => 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;
@ -722,15 +722,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 '.