Bug 42847: (QA follow-up) Clear session state on all login_core exit paths

login_core only cleared leftover session state on the failed-auth
branch. The earlier 'SIP access not authorized' return (unknown
terminal id) left any account/ils/institution/policy/credentials from
a prior connection on a reused prefork worker in place.

Move the reset to the top of login_core so every non-success exit path
leaves clean state, and only a successful api_auth re-establishes the
session. This keeps the no-authenticated-state-until-auth invariant in
a single place rather than duplicating the key list per branch.

Also extend the unit test to cover the unknown-terminal path on a
reused worker.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit c375a5cd34e88b0a1d2d0109a523199ef9a6dc06)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-06-19 10:29:55 +01:00 committed by Lucas Gass
parent e00a821a26
commit 6dfe5f4415
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 18 additions and 5 deletions

View File

@ -932,6 +932,11 @@ sub login_core {
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 );
# Check if this userid is authorized for SIP access
if ( !exists( $server->{config}->{accounts}->{$uid} ) ) {
siplog( "LOG_WARNING", "MsgType::login_core: SIP access not authorized for user '$uid'" );
@ -948,10 +953,6 @@ sub login_core {
"LOG_WARNING", "Authentication failed for SIP terminal '%s' of '%s': %s",
$uid, $inst, ( $auth_status || 'unknown' )
);
# Clear any session state, including anything left over from a previous connection on
# this preforked worker, so the caller can't act as an authenticated terminal
delete $server->{$_} foreach qw( account ils institution policy sip_username sip_password );
return 0;
}

View File

@ -352,7 +352,7 @@ subtest 'message parsing arrayref prevention' => sub {
};
subtest 'login_core() authentication tests' => sub {
plan tests => 9;
plan tests => 12;
# A failed login must never leave the connection in an authenticated state.
my $uid = 'sip_bug_42847';
@ -396,4 +396,16 @@ subtest 'login_core() authentication tests' => sub {
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' );
};