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

Bug 32178: Remove security breach introduced in bug 31378 

This patch removes a security breach in C4::Auth::check_api_auth introduced by bug 31378, where when someone called an api with the parameters userid and auth_client_login, check_api_auth would automatically asume the user calling was that userid.

This patch also introduces C4::Auth::create_basic_session(), a function that creates a session and adds the minimum basic parameters.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Agustin Moyano 2022-11-11 22:22:23 +00:00 committed by Tomas Cohen Arazi
parent 7a413f8015
commit ca57674700
No known key found for this signature in database
GPG Key ID: 0A272EA1B2F3C15F
4 changed files with 64 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
C4/Auth.pm
View File

@ -74,7 +74,7 @@ BEGIN {
@EXPORT_OK = qw(
checkauth check_api_auth get_session check_cookie_auth checkpw checkpw_internal checkpw_hash
get_all_subpermissions get_user_subpermissions track_login_daily in_iprange
get_template_and_user haspermission
get_template_and_user haspermission create_basic_session
);
$ldap = C4::Context->config('useldapserver') || 0;
@ -1561,7 +1561,6 @@ sub check_api_auth {
# new login
my $userid = $query->param('userid');
my $password = $query->param('password');
my $auth_client_login = $query->param('auth_client_login');
my ( $return, $cardnumber, $cas_ticket );
# Proxy CAS auth
@ -1571,9 +1570,6 @@ sub check_api_auth {
# In case of a CAS authentication, we use the ticket instead of the password
my $PT = $query->param('PT');
( $return, $cardnumber, $userid, $cas_ticket ) = check_api_auth_cas( $PT, $query ); # EXTERNAL AUTH
} elsif ( $auth_client_login && ( $userid || $query->param('cardnumber') ) ) {
$cardnumber = $query->param('cardnumber');
$return = 1;
} else {
# User / password auth
@ -1871,6 +1867,37 @@ sub get_session {
return $session;
}
=head2 create_basic_session
my $session = create_basic_session({ patron => $patron, interface => $interface });
Creates a session and adds all basic parameters for a session to work
=cut
sub create_basic_session {
my $params = shift;
my $patron = $params->{patron};
my $interface = $params->{interface};
my $session = get_session("");
$session->param( 'number', $patron->borrowernumber );
$session->param( 'id', $patron->userid );
$session->param( 'cardnumber', $patron->cardnumber );
$session->param( 'firstname', $patron->firstname );
$session->param( 'surname', $patron->surname );
$session->param( 'branch', $patron->branchcode );
$session->param( 'branchname', $patron->library->branchname );
$session->param( 'flags', $patron->flags );
$session->param( 'emailaddress', $patron->email );
$session->param( 'ip', $session->remote_addr() );
$session->param( 'lasttime', time() );
$session->param( 'interface', $interface);
return $session;
}
# FIXME no_set_userenv may be replaced with force_branchcode_for_userenv
# (or something similar)

23
Koha/REST/Plugin/Auth/IdP.pm
View File

@ -81,24 +81,15 @@ Generates a new session.
$app->helper(
'auth.session' => sub {
my ( $c, $patron ) = @_;
my $userid = $patron->userid;
my $cardnumber = $patron->cardnumber;
my $cgi = CGI->new;
my ( $c, $params ) = @_;
my $patron = $params->{patron};
my $interface = $params->{interface};
my $provider = $params->{provider};
$cgi->param( userid => $userid );
$cgi->param( cardnumber => $cardnumber );
$cgi->param( auth_client_login => 1 );
my $session = C4::Auth::create_basic_session({ patron => $patron, interface => $interface });
$session->param('idp_code', $provider);
my ( $status, $cookie, $session_id ) = C4::Auth::check_api_auth($cgi);
Koha::Exceptions::UnderMaintenance->throw( code => 503 )
if $status eq "maintenance";
Koha::Exceptions::Auth::CannotCreateSession->throw( code => 500 )
unless $status eq "ok";
return ( $status, $cookie, $session_id );
return $session->id;
}
);
}

4
Koha/REST/V1/OAuth/Client.pm
View File

@ -65,7 +65,7 @@ sub login {
$uri = '/cgi-bin/koha/mainpage.pl';
}
unless ( $provider_config ) {
unless ( $provider_config && $provider_config->{authorize_url} ) {
my $error = "No configuration found for your provider";
return $c->redirect_to($uri."?auth_error=$error");
}
@ -99,7 +99,7 @@ sub login {
);
}
my ( $status, $cookie, $session_id ) = $c->auth->session($patron);
my $session_id = $c->auth->session({ patron => $patron, interface => $interface, provider => $provider });
$c->cookie( CGISESSID => $session_id, { path => "/" } );

24
t/db_dependent/Auth.t
View File

@ -10,7 +10,7 @@ use CGI qw ( -utf8 );
use Test::MockObject;
use Test::MockModule;
use List::MoreUtils qw/all any none/;
use Test::More tests => 16;
use Test::More tests => 17;
use Test::Warn;
use t::lib::Mocks;
use t::lib::TestBuilder;
@ -833,4 +833,26 @@ subtest 'Userenv clearing in check_cookie_auth' => sub {
is( C4::Context->userenv, undef, 'Environment should be cleared again' );
};
subtest 'create_basic_session tests' => sub {
plan tests => 12;
my $patron = $builder->build_object({ class => 'Koha::Patrons' });
my $interface = 'opac';
my $session = C4::Auth::create_basic_session({ patron => $patron, interface => $interface });
isnt($session->id, undef, 'A new sessionID was created');
is( $session->param('number'), $patron->borrowernumber, 'Session parameter number matches' );
is( $session->param('id'), $patron->userid, 'Session parameter id matches' );
is( $session->param('cardnumber'), $patron->cardnumber, 'Session parameter cardnumber matches' );
is( $session->param('firstname'), $patron->firstname, 'Session parameter firstname matches' );
is( $session->param('surname'), $patron->surname, 'Session parameter surname matches' );
is( $session->param('branch'), $patron->branchcode, 'Session parameter branch matches' );
is( $session->param('branchname'), $patron->library->branchname, 'Session parameter branchname matches' );
is( $session->param('flags'), $patron->flags, 'Session parameter flags matches' );
is( $session->param('emailaddress'), $patron->email, 'Session parameter emailaddress matches' );
is( $session->param('ip'), $session->remote_addr(), 'Session parameter ip matches' );
is( $session->param('interface'), $interface, 'Session parameter interface matches' );
};
$schema->storage->txn_rollback;