Bug 31250: Deny clearing cookies with numeric suffix
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Thu, 28 Jul 2022 12:43:20 +0000 (12:43 +0000)
committerLucas Gass <lucas@bywatersolutions.com>
Mon, 31 Oct 2022 20:44:29 +0000 (20:44 +0000)
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 <m.de.rooy@rijksmuseum.nl>
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 <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit cb036faf2c534a14708b517cea40d98fc6e344c2)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Koha/CookieManager.pm
t/CookieManager.t

index 018c806..0c665ef 100644 (file)
@@ -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
index 1efc80e..0011e9f 100755 (executable)
@@ -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 {