gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42847: [24.11] Prevent previously authenticated user (squashed) 

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
This commit is contained in:
Kyle M Hall 2026-06-16 10:12:07 -04:00 committed by Wainui Witika-Park
parent afdbbd117e
commit 498c85b62a
No known key found for this signature in database
GPG Key ID: 2D0A6AB5DAF1B78D
3 changed files with 80 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
C4/SIP/SIPServer.pm
View File

@ -236,6 +236,7 @@ sub process_request {
Koha::Caches->flush_L1_caches();
$self->{account} = undef; # Clear out the account from the last request, it may be different
$self->{ils} = undef; # And the ILS handle, so a reused worker can't reuse a prior session's connection
$self->{logger} = set_logger( Koha::Logger->get( { interface => 'sip' } ) );
# Flush previous MDCs to prevent accidentally leaking incorrect MDC-entries
@ -400,11 +401,10 @@ sub telnet_transport {
if ( exists( $config->{accounts}->{$uid} )
&& ( $pwd eq $config->{accounts}->{$uid}->{password} ) )
&& C4::SIP::Sip::MsgType::login_core( $self, $uid, $pwd ) )
{
$account = $config->{accounts}->{$uid};
if ( C4::SIP::Sip::MsgType::login_core( $self, $uid, $pwd ) ) {
last;
}
last;
}
siplog( "LOG_WARNING", "Invalid login attempt: '%s'", ( $uid || '' ) );
print("Invalid login$CRLF");

23
C4/SIP/Sip/MsgType.pm
View File

@ -927,6 +927,13 @@ sub login_core {
my $uid = shift;
my $pwd = shift;
my $status = 1; # Assume it all works
# Clear any session state up front, including anything left over from a previous connection
# on this preforked worker, so no login_core exit path can leave the caller acting as an
# authenticated terminal. Only a successful api_auth below re-establishes the session.
delete $server->{$_} foreach qw( account ils institution policy sip_username sip_password );
if ( !exists( $server->{config}->{accounts}->{$uid} ) ) {
siplog( "LOG_WARNING", "MsgType::login_core: Unknown login '$uid'" );
$status = 0;
@ -935,13 +942,8 @@ sub login_core {
$status = 0;
} else {
# Store the active account someplace handy for everybody else to find.
$server->{account} = $server->{config}->{accounts}->{$uid};
my $inst = $server->{account}->{institution};
$server->{institution} = $server->{config}->{institutions}->{$inst};
$server->{policy} = $server->{institution}->{policy};
$server->{sip_username} = $uid;
$server->{sip_password} = $pwd;
# Authenticate before storing the account on the session
my $inst = $server->{config}->{accounts}->{$uid}->{institution};
my $auth_status = api_auth( $uid, $pwd, $inst );
if ( !$auth_status or $auth_status !~ /^ok$/i ) {
@ -951,6 +953,13 @@ sub login_core {
);
$status = 0;
} else {
# Store the active account configuration now user has authenticated
$server->{account} = $server->{config}->{accounts}->{$uid};
$server->{institution} = $server->{config}->{institutions}->{$inst};
$server->{policy} = $server->{institution}->{policy};
$server->{sip_username} = $uid;
$server->{sip_password} = $pwd;
siplog( "LOG_INFO", "Successful login/auth for '%s' of '%s'", $server->{account}->{id}, $inst );
#

62
t/db_dependent/SIP/MsgType.t
View File

@ -19,13 +19,14 @@
use Modern::Perl;
use Test::More tests => 3;
use Test::More tests => 4;
use Test::NoWarnings;
use Test::Warn;
use Test::MockModule;
use C4::SIP::Sip::MsgType;
use C4::SIP::Sip::Constants qw(:all);
use UNIVERSAL::require;
use t::lib::TestBuilder;
use t::lib::Mocks;
@ -349,3 +350,62 @@ subtest 'message parsing arrayref prevention' => sub {
$schema->storage->txn_rollback;
};
subtest 'login_core() authentication tests' => sub {
plan tests => 12;
# A failed login must never leave the connection in an authenticated state.
my $uid = 'sip_bug_42847';
my $build_server = sub {
return {
config => {
accounts => { $uid => { id => $uid, institution => 'CPL' } },
institutions => { CPL => { id => 'CPL', implementation => 'ILS', policy => {} } },
},
};
};
my $auth_status = 'ok';
my $mock_auth = Test::MockModule->new('C4::SIP::Sip::MsgType');
$mock_auth->mock( 'api_auth', sub { return $auth_status; } );
my $mock_ils = Test::MockModule->new('C4::SIP::ILS');
$mock_ils->mock( 'new', sub { return bless {}, 'C4::SIP::ILS'; } );
# Successful login stores the account and an ILS handle
$auth_status = 'ok';
my $server = $build_server->();
ok( C4::SIP::Sip::MsgType::login_core( $server, $uid, 'goodpw' ), 'login_core returns true on successful auth' );
is( $server->{account}->{id}, $uid, 'account stored on the session after a successful login' );
isa_ok( $server->{ils}, 'C4::SIP::ILS', 'ILS handle built after a successful login' );
# A wrong password must not store the account
$auth_status = 'failed';
$server = $build_server->();
ok( !C4::SIP::Sip::MsgType::login_core( $server, $uid, 'badpw' ), 'login_core returns false on failed auth' );
is( $server->{account}, undef, 'account is not stored on the session after a failed login' );
is( $server->{ils}, undef, 'no ILS handle after a failed login' );
# A reused prefork worker that handled a successful login must not let the
# next connection inherit that session when its login fails
$auth_status = 'ok';
$server = $build_server->();
C4::SIP::Sip::MsgType::login_core( $server, $uid, 'goodpw' ); # previous session on this worker
$auth_status = 'failed';
ok( !C4::SIP::Sip::MsgType::login_core( $server, $uid, 'badpw' ), 'failed login on a reused worker returns false' );
is( $server->{account}, undef, 'leftover account cleared after a failed login on a reused worker' );
is( $server->{ils}, undef, 'leftover ILS handle cleared after a failed login on a reused worker' );
# A reused worker whose next connection presents an unknown terminal id must
# also have the leftover session cleared, not just the wrong-password case
$auth_status = 'ok';
$server = $build_server->();
C4::SIP::Sip::MsgType::login_core( $server, $uid, 'goodpw' ); # previous session on this worker
ok(
!C4::SIP::Sip::MsgType::login_core( $server, 'unknown_terminal', 'badpw' ),
'login for an unknown terminal returns false'
);
is( $server->{account}, undef, 'leftover account cleared after an unknown-terminal login on a reused worker' );
is( $server->{ils}, undef, 'leftover ILS handle cleared after an unknown-terminal login on a reused worker' );
};