Attachment 140164 Details for Bug 31250 – Bug 31250: Deny clearing cookies with numeric suffix

[patch] Bug 31250: Deny clearing cookies with numeric suffix

Bug-31250-Deny-clearing-cookies-with-numeric-suffi.patch (text/plain), 3.33 KB, created by Kyle M Hall (khall) on 2022-09-02 16:06:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2022-09-02 16:06:15 UTC

Size: 3.33 KB

patch

obsolete

>From d99c7e8962dece4c4d8a0af72c857495121b4675 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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 <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>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 31250: 138203 | 138204 | 138981 | 138982 | 139389 | 139571 | 140163 | 140164 | 140165 | 140166