From d99c7e8962dece4c4d8a0af72c857495121b4675 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 28 Jul 2022 12:43:20 +0000 Subject: [PATCH] Bug 31250: Deny clearing cookies with numeric suffix This change allows us to add catalogue_editor_ to the deny list in koha-conf.xml and keep cookies like catalogue_editor_123. Test plan: Run t/CookieManager.t Signed-off-by: Marcel de Rooy To test: 1 - Sign in to staff client 2 - Search for records and edit one 3 - Switch to advanced editor 4 - View cookies (F12/developer panel/storage tab) 5 - Note cookie like 'catalogue_editor_##' with value 'advanced' 6 - Log out 7 - note cookie value deleted 8 - Log in and search/edit a record 9 - Basic editor loads 10 - Apply patch 11 - Add line to koha-conf as described in second patch 12 - Restart all 13 - Switch to advanced editor 14 - Cookie value updated 15 - Logout 16 - Cookie value remains 17 - Log in and search/edit 18 - Confirm advanced editor loads Signed-off-by: Nick Clemens Signed-off-by: Kyle M Hall --- Koha/CookieManager.pm | 6 +++++- t/CookieManager.t | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Koha/CookieManager.pm b/Koha/CookieManager.pm index 018c8068e1..0c665efc61 100644 --- a/Koha/CookieManager.pm +++ b/Koha/CookieManager.pm @@ -101,7 +101,11 @@ sub clear_unless { } next if !$name; - if( !$self->{_remove_unless}->{$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} ) { next if $seen->{$name}; push @rv, CGI::Cookie->new( # -expires explicitly omitted to create shortlived 'session' cookie diff --git a/t/CookieManager.t b/t/CookieManager.t index 1efc80e37a..0011e9f118 100755 --- a/t/CookieManager.t +++ b/t/CookieManager.t @@ -41,7 +41,7 @@ subtest 'new' => sub { }; subtest 'clear_unless' => sub { - plan tests => 15; + plan tests => 16; t::lib::Mocks::mock_config( Koha::CookieManager::DENY_LIST_VAR, [ 'aap', 'noot' ] ); @@ -73,6 +73,20 @@ subtest 'clear_unless' => sub { is( $rv[4]->value, q{}, 'zus empty' ); 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_' ] ); + $cmgr = Koha::CookieManager->new; + $cookie1 = $q->cookie( -name => 'catalogue_editor_234', -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' ); + + $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' ); }; subtest 'replace_in_list' => sub { -- 2.30.2