Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 31250: (QA follow-up) The future will be easier

Browse source 
As requested by Jonathan, we need more flexibility ;)
Here it comes.

Test plan:
Run t/CookieManager.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit cffd36bcb4)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Marcel de Rooy 2022-08-18 14:32:46 +00:00 committed by Lucas Gass
parent bf7a65e13e
commit d865ce1665
4 changed files with 40 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
Koha/CookieManager.pm
View file

@ -101,11 +101,7 @@ sub clear_unless {
}
next if !$name;
# Try stripping _\d+ from name for cookiea like catalogue_editor_123
my $stripped_name = $name;
$stripped_name =~ s/_\d+$/_/;
if( !$self->{_remove_unless}->{$stripped_name} && !$self->{_remove_unless}->{$name} ) {
if( $self->_should_be_cleared($name) ) {
next if $seen->{$name};
push @rv, CGI::Cookie->new(
# -expires explicitly omitted to create shortlived 'session' cookie
@ -122,6 +118,21 @@ sub clear_unless {
return \@rv;
}
sub _should_be_cleared { # when it is not on the deny list in koha-conf
my ( $self, $name ) = @_;
return if $self->{_remove_unless}->{$name}; # exact match
# Now try the entries as regex
foreach my $k ( keys %{$self->{_remove_unless}} ) {
my $reg = $self->{_remove_unless}->{$k};
# The entry in koha-conf should match the complete string
# So adding a ^ and $
return if $name =~ qr/^${k}$/;
}
return 1;
}
=head2 replace_in_list
$list2 = $mgr->replace_in_list( $list1, $cookie );

3
debian/templates/koha-conf-site.xml.in vendored
View file

@ -457,9 +457,10 @@ __END_SRU_PUBLICSERVER__
</message_broker>
<do_not_remove_cookie>__KEEP_COOKIE__</do_not_remove_cookie>
<do_not_remove_cookie>catalogue_editor_</do_not_remove_cookie>
<do_not_remove_cookie>catalogue_editor_\d+</do_not_remove_cookie>
<!-- Uncomment lines like hereunder to not clear cookies at logout:
The cookie name is case sensitive.
NOTE: You may use regex constructions like the example above.
<do_not_remove_cookie>KohaOpacLanguage</do_not_remove_cookie>
-->

3
etc/koha-conf.xml
View file

@ -273,9 +273,10 @@
<vhost></vhost>
</message_broker>
<do_not_remove_cookie>catalogue_editor_</do_not_remove_cookie>
<do_not_remove_cookie>catalogue_editor_\d+</do_not_remove_cookie>
<!-- Uncomment lines like hereunder to not clear cookies at logout:
The cookie name is case sensitive.
NOTE: You may use regex constructions like the example above.
<do_not_remove_cookie>KohaOpacLanguage</do_not_remove_cookie>
-->

27
t/CookieManager.t
View file

@ -41,7 +41,7 @@ subtest 'new' => sub {
};
subtest 'clear_unless' => sub {
plan tests => 16;
plan tests => 17;
t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'aap', 'noot' ] );
@ -74,19 +74,32 @@ subtest 'clear_unless' => sub {
is( $rv[1]->httponly, 0, 'cleared wim is not httponly' );
is( $rv[2]->httponly, 1, 'aap httponly' );
# Test with _123 prefix
t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'catalogue_editor_' ] );
# Test with numeric suffix (via regex)
t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'catalogue_editor_\d+' ] );
$cmgr = Koha::CookieManager->new;
$cookie1 = $q->cookie( -name => 'catalogue_editor_234', -value => '1', -expires => '+1y' );
$cookie1 = $q->cookie( -name => 'catalogue_editor_abc', -value => '1', -expires => '+1y' );
$cookie2 = $q->cookie( -name => 'catalogue_editor_345', -value => '1', -expires => '+1y' );
$cookie3 = $q->cookie( -name => 'catalogue_editor_', -value => '1', -expires => '+1y' );
$cookie4 = $q->cookie( -name => 'catalogue_editor', -value => '1', -expires => '+1y' );
$cookie4 = $q->cookie( -name => 'catalogue_editor_123x', -value => '1', -expires => '+1y' );
$list = [ $cookie1, $cookie2, $cookie3, $cookie4 ];
@rv = @{$cmgr->clear_unless( @$list )};
is_deeply( [ map { $_->value ? $_->name : () } @rv ],
[ 'catalogue_editor_234', 'catalogue_editor_345', 'catalogue_editor_' ],
'Only cookie4 should have been cleared' );
[ 'catalogue_editor_345' ],
'Cookie2 should be found only' );
# Test with another regex (yes, highly realistic examples :)
t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'next_\w+_number\d{2}_(now|never)' ] );
$cmgr = Koha::CookieManager->new;
my $cookie5;
$cookie1 = $q->cookie( -name => 'next_mynewword_number99_never', -value => '1', -expires => '+1y' ); #fine
$cookie2 = $q->cookie( -name => 'prefixed_next_mynewword_number99_never', -value => '1', -expires => '+1y' ); # wrong prefix
$cookie3 = $q->cookie( -name => 'next_mynew-word_number99_never', -value => '1', -expires => '+1y' ); # wrong: hyphen in word
$cookie4 = $q->cookie( -name => 'mynewword_number999_never', -value => '1', -expires => '+1y' ); # wrong: three digits
$cookie5 = $q->cookie( -name => 'next_mynewword_number99_always', -value => '1', -expires => '+1y' ); # wrong: always
@rv = @{$cmgr->clear_unless( $cookie1, $cookie2, $cookie3, $cookie4, $cookie5 )};
is_deeply( [ map { $_->value ? $_->name : () } @rv ], [ 'next_mynewword_number99_never' ], 'Only cookie1 matched' );
};
subtest 'replace_in_list' => sub {