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

Bug 25360: Use secure flag for CGISESSID cookie when using HTTPS 

This patch adds the secure flag to the CGISESSID cookie when using HTTPS.
This prevents the cookie being used again over a normal HTTP
request.

Bug 25360: [Follow-up] Test for "on" or "ON" value for HTTPS env var

This patch tests for HTTPS "on" or "ON" before setting the secure
cookie.

Bug 25360: [Follow-up] Fix typo in C4/InstallAuth.pm

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Amended number of tests in Context.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
This commit is contained in:
David Cook 2020-05-04 11:12:26 +10:00 committed by Aleisha Amohia
parent cd8ecbfed0
commit 2e544f127c
4 changed files with 61 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
C4/Auth.pm
View File

@ -228,6 +228,7 @@ sub get_template_and_user {
-value => '',
-expires => '',
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$template->param(
@ -830,6 +831,7 @@ sub checkauth {
-value => '',
-expires => '',
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$loggedin = 1;
}
@ -930,7 +932,8 @@ sub checkauth {
$cookie = $query->cookie(
-name => 'CGISESSID',
-value => $session->id,
-HttpOnly => 1
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$session->param( 'lasttime', time() );
unless ( $sessiontype && $sessiontype eq 'anon' ) { #if this is an anonymous session, we want to update the session, but not behave as if they are logged in...
@ -959,7 +962,8 @@ sub checkauth {
$cookie = $query->cookie(
-name => 'CGISESSID',
-value => $session->id,
-HttpOnly => 1
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
my $pki_field = C4::Context->preference('AllowPKIAuth');
if ( !defined($pki_field) ) {
@ -1127,7 +1131,8 @@ sub checkauth {
$cookie = $query->cookie(
-name => 'CGISESSID',
-value => '',
-HttpOnly => 1
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$info{'wrongip'} = 1;
}
@ -1207,7 +1212,8 @@ sub checkauth {
$cookie = $query->cookie(
-name => 'CGISESSID',
-value => '',
-HttpOnly => 1
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
}
@ -1475,6 +1481,7 @@ sub check_api_auth {
-name => 'CGISESSID',
-value => $session->id,
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$session->param( 'lasttime', time() );
my $flags = haspermission( $userid, $flagsrequired );
@ -1529,6 +1536,7 @@ sub check_api_auth {
-name => 'CGISESSID',
-value => $sessionID,
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
if ( $return == 1 ) {
my (

26
C4/Context.pm
View File

@ -1119,6 +1119,32 @@ sub set_remote_address {
}
}
=head3 https_enabled
https_enabled should be called when checking if a HTTPS connection
is used.
Note that this depends on a HTTPS environmental variable being defined
by the web server. This function may not return the expected result,
if your web server or reverse proxies are not setting the correct
X-Forwarded-Proto headers and HTTPS environmental variable.
Note too that the HTTPS value can vary from web server to web server.
We are relying on the convention of the value being "on" or "ON" here.
=cut
sub https_enabled {
my $https_enabled = 0;
my $env_https = $ENV{HTTPS};
if ($env_https){
if ($env_https =~ /^ON$/i){
$https_enabled = 1;
}
}
return $https_enabled;
}
1;
__END__

8
C4/InstallAuth.pm
View File

@ -267,6 +267,7 @@ sub checkauth {
-name => 'CGISESSID',
-value => $session->id,
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
$loggedin = 1;
$userid = $session->param('cardnumber');
@ -307,6 +308,7 @@ sub checkauth {
-name => 'CGISESSID',
-value => $sessionID,
-HttpOnly => 1,
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
if ( $return == 2 ) {
@ -352,7 +354,8 @@ sub checkauth {
-name => 'CGISESSID',
-value => '',
-HttpOnly => 1,
-expires => ''
-expires => '',
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
}
if ($envcookie) {
@ -394,7 +397,8 @@ sub checkauth {
-name => 'CGISESSID',
-value => $sessionID,
-HttpOnly => 1,
-expires => ''
-expires => '',
-secure => ( C4::Context->https_enabled() ? 1 : 0 ),
);
print $query->header(
-type => 'text/html; charset=utf-8',

18
t/Context.t
View File

@ -18,7 +18,7 @@
use Modern::Perl;
use DBI;
use Test::More tests => 28;
use Test::More tests => 34;
use Test::MockModule;
use Test::Warn;
use YAML;
@ -104,3 +104,19 @@ is(C4::Context->interface, 'opac', 'interface still opac');
is( C4::Context->interface( 'SiP' ), 'sip', 'interface SiP' );
is( C4::Context->interface( 'COMMANDLINE' ), 'commandline', 'interface commandline uc' );
is( C4::Context->interface( 'CRON' ), 'cron', 'interface cron uc' );
{
local %ENV = %ENV;
delete $ENV{HTTPS};
is( C4::Context->https_enabled, 0, "Undefined HTTPS env returns 0");
$ENV{HTTPS} = '1';
is( C4::Context->https_enabled, 0, "Invalid 1 HTTPS env returns 0");
$ENV{HTTPS} = 'off';
is( C4::Context->https_enabled, 0, "off HTTPS env returns 0");
$ENV{HTTPS} = 'OFF';
is( C4::Context->https_enabled, 0, "OFF HTTPS env returns 0");
$ENV{HTTPS} = 'on';
is( C4::Context->https_enabled, 1, "on HTTPS env returns 1");
$ENV{HTTPS} = 'ON';
is( C4::Context->https_enabled, 1, "ON HTTPS env returns 1");
}