From c3624cf8ef2c36b5e23a12f4267d19d65c40c3f7 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Thu, 18 Aug 2022 14:32:46 +0000
Subject: [PATCH] Bug 31250: (QA follow-up) The future will be easier
Content-Type: text/plain; charset=utf-8

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>
---
 Koha/CookieManager.pm                  | 20 +++++++++++++++-----
 debian/templates/koha-conf-site.xml.in |  3 ++-
 etc/koha-conf.xml                      |  3 ++-
 t/CookieManager.t                      | 23 ++++++++++++++++++-----
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/Koha/CookieManager.pm b/Koha/CookieManager.pm
index 0c665efc61..b3e383d5b7 100644
--- a/Koha/CookieManager.pm
+++ b/Koha/CookieManager.pm
@@ -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,20 @@ 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 is always considered to be from the start
+        return if $name =~ qr/^${k}/;
+    }
+    return 1;
+}
+
 =head2 replace_in_list
 
     $list2 = $mgr->replace_in_list( $list1, $cookie );
diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in
index bf77b0839f..bcf6bab0f6 100644
--- a/debian/templates/koha-conf-site.xml.in
+++ b/debian/templates/koha-conf-site.xml.in
@@ -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>
  -->
 
diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml
index dae6d96c39..793420437d 100644
--- a/etc/koha-conf.xml
+++ b/etc/koha-conf.xml
@@ -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>
  -->
 
diff --git a/t/CookieManager.t b/t/CookieManager.t
index 0011e9f118..e9954799bd 100755
--- a/t/CookieManager.t
+++ b/t/CookieManager.t
@@ -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,8 +74,8 @@ 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' );
     $cookie2 = $q->cookie( -name => 'catalogue_editor_345', -value => '1', -expires => '+1y' );
@@ -85,8 +85,21 @@ subtest 'clear_unless' => sub {
     $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_234', 'catalogue_editor_345' ],
+        'Cookie3 and cookie4 should have been cleared' );
+
+    # 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_neverrr', -value => '1', -expires => '+1y' ); #fine
+    $cookie2 = $q->cookie( -name => 'prefixed_next_mynewword_number99_never_better_123', -value => '1', -expires => '+1y' ); # wrong prefix
+    $cookie3 = $q->cookie( -name => 'next_mynew-word_number99_never_better_123', -value => '1', -expires => '+1y' ); # wrong: hyphen in word
+    $cookie4 = $q->cookie( -name => 'mynewword_number999_never_better_123', -value => '1', -expires => '+1y' ); # wrong: three digits
+    $cookie5 = $q->cookie( -name => 'next_mynewword_number99_nooit_meer', -value => '1', -expires => '+1y' ); # wrong: nooit
+    @rv = @{$cmgr->clear_unless( $cookie1, $cookie2, $cookie3, $cookie4, $cookie5 )};
+    is_deeply( [ map { $_->value ? $_->name : () } @rv ], [ 'next_mynewword_number99_neverrr' ], 'Only cookie1' );
+
 };
 
 subtest 'replace_in_list' => sub {
-- 
2.20.1