Attachment 133212 Details for Bug 26019 – Bug 26019: Koha should set SameSite attribute on cookies

[patch] Bug 26019: Koha should set SameSite attribute on cookies

Bug-26019-Koha-should-set-SameSite-attribute-on-co.patch (text/plain), 18.90 KB, created by Owen Leonard on 2022-04-12 12:22:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2022-04-12 12:22:56 UTC

Size: 18.90 KB

patch

obsolete

>From 17aa1707f1e3e86a01c3cde1c8311265bc749829 Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Fri, 11 Feb 2022 15:51:49 +0000
>Subject: [PATCH] Bug 26019: Koha should set SameSite attribute on cookies
>
>This patch modifies the way Koha sets cookies so that the "sameSite"
>attribute is explicitly set to "Lax." This option is chosen because it
>is the value which is currently assumed by browsers when the sameSite
>attribute is not set.
>
>To test, apply the patch and restart services.
>
>- Log in to the staff interface and open your browser's developer tools.
>  - In Firefox, look for a "Storage" tab.
>  - In Chrome, look for an "Application" tab.
>- Under "Cookies," click the URL of the staff interface.
>- You should see all the cookies which are set for that domain.
>- The CGISESSID cookie should have sameSite set to "Lax."
>
>- Go to Cataloging -> New record.
>  - Check the "marcdocs" and "marctags" cookies.
>- Switch to the Advanced MARC editor (you may need to enable
>  theEnableAdvancedCatalogingEditor preference).
>  - Check the "catalogue_editor" cookie.
>- Add a new item to an existing bibliographic record.
>  - Check the "LastCreatedItem" cookie which is set after you save the
>    new item.
>- Go to Authorities -> Authority search.
>  - In authority search results, click "Merge" from the "Actions" menu
>    next to one of the results..
>    - Check the "auth_to_merge" cookie.
>- Go to Administration -> MARC bibliographic framework
>  - Choose "MARC structure" from the menu corresponding to one of the
>    frameworks.
>  - Check the "Display only used tags/subfields" checkbox.
>    - Check the "marctagstructure_selectdisplay" cookie.
>- Go to Circulation -> Check out to a patron with checkouts.
>  - Check the "Always show checkouts immediately" checkbox.
>    - Check the "issues-table-load-immediately-circulation" cookie.
>- Go to Tools -> Patron clubs. You will need at least one active club
>  with one or more patrons enrolled.
>  - From the list of clubs, click Actions -> Search to hold.
>    - Check the "holdforclub" cookie.
>- Go to Tools -> Batch item modification and submit a batch of items.
>  - Uncheck one or more checkboxes in the "Show/hide columns" area.
>    - Check the "showColumns" cookie.
>- View a patron -> Search to hold.
>  - Check the 'holdfor' cookie.
>- With WebBasedSelfCheck enabled, log in to the self-checkout page.
>  - Check the "JWT" cookie.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> C4/Auth.pm                                            | 11 ++++++++++-
> C4/InstallAuth.pm                                     |  4 ++++
> C4/Templates.pm                                       |  1 +
> cataloguing/additem.pl                                |  3 ++-
> .../intranet-tmpl/prog/en/includes/authorities_js.inc |  2 +-
> .../intranet-tmpl/prog/en/includes/cateditor-ui.inc   |  2 +-
> .../prog/en/modules/admin/marctagstructure.tt         |  2 +-
> .../prog/en/modules/cataloguing/addbiblio.tt          | 10 +++++-----
> .../prog/en/modules/cataloguing/addbooks.tt           |  2 +-
> .../intranet-tmpl/prog/en/modules/clubs/clubs.tt      |  2 +-
> koha-tmpl/intranet-tmpl/prog/js/acquisitions-menu.js  |  2 +-
> koha-tmpl/intranet-tmpl/prog/js/checkouts.js          |  2 +-
> koha-tmpl/intranet-tmpl/prog/js/members-menu.js       |  2 +-
> koha-tmpl/intranet-tmpl/prog/js/pages/batchMod.js     |  8 ++++----
> opac/sco/sco-main.pl                                  |  1 +
> 15 files changed, 35 insertions(+), 19 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index ba28d58cd2..0714861115 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -252,6 +252,7 @@ sub get_template_and_user {
>                 -value    => '',
>                 -HttpOnly => 1,
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax',
>             ));
> 
>             $template->param(
>@@ -874,6 +875,7 @@ sub checkauth {
>             -value    => '',
>             -HttpOnly => 1,
>             -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+            -sameSite => 'Lax',
>         ));
>         $loggedin = 1;
>     }
>@@ -932,6 +934,7 @@ sub checkauth {
>                     -value    => $session->id,
>                     -HttpOnly => 1,
>                     -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                    -sameSite => 'Lax',
>                 ));
> 
>                 $flags = haspermission( $userid, $flagsrequired );
>@@ -976,6 +979,7 @@ sub checkauth {
>             -value    => $sessionID,
>             -HttpOnly => 1,
>             -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+            -sameSite => 'Lax',
>         ));
>         my $pki_field = C4::Context->preference('AllowPKIAuth');
>         if ( !defined($pki_field) ) {
>@@ -1176,6 +1180,7 @@ sub checkauth {
>                                 -value    => '',
>                                 -HttpOnly => 1,
>                                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                                -sameSite => 'Lax',
>                             ));
>                             $info{'wrongip'} = 1;
>                         }
>@@ -1265,6 +1270,7 @@ sub checkauth {
>                 -value    => '',
>                 -HttpOnly => 1,
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax',
>             ));
>         }
> 
>@@ -1417,7 +1423,8 @@ sub checkauth {
>         {   type              => 'text/html',
>             charset           => 'utf-8',
>             cookie            => $cookie,
>-            'X-Frame-Options' => 'SAMEORIGIN'
>+            'X-Frame-Options' => 'SAMEORIGIN',
>+            -sameSite => 'Lax'
>         }
>       ),
>       $template->output;
>@@ -1500,6 +1507,7 @@ sub check_api_auth {
>             -value    => $session->id,
>             -HttpOnly => 1,
>             -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+            -sameSite => 'Lax'
>         );
>         return ( $return, $cookie, $session ); # return == 'ok' here
> 
>@@ -1540,6 +1548,7 @@ sub check_api_auth {
>                 -value    => $sessionID,
>                 -HttpOnly => 1,
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax'
>             );
>             if ( $return == 1 ) {
>                 my (
>diff --git a/C4/InstallAuth.pm b/C4/InstallAuth.pm
>index de19adc4f0..56dd8e3260 100644
>--- a/C4/InstallAuth.pm
>+++ b/C4/InstallAuth.pm
>@@ -262,6 +262,7 @@ sub checkauth {
>                 -value    => $session->id,
>                 -HttpOnly => 1,
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax'
>             );
>             $loggedin = 1;
>             $userid   = $session->param('cardnumber');
>@@ -302,6 +303,7 @@ sub checkauth {
>                 -value    => $sessionID,
>                 -HttpOnly => 1,
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax'
>             );
>             if ( $return == 2 ) {
> 
>@@ -349,6 +351,7 @@ sub checkauth {
>                 -HttpOnly => 1,
>                 -expires => '',
>                 -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+                -sameSite => 'Lax'
>             );
>         }
>         if ($envcookie) {
>@@ -392,6 +395,7 @@ sub checkauth {
>         -HttpOnly => 1,
>         -expires => '',
>         -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+        -sameSite => 'Lax'
>     );
>     print $query->header(
>         -type    => 'text/html; charset=utf-8',
>diff --git a/C4/Templates.pm b/C4/Templates.pm
>index 1d0156837b..789495f3fa 100644
>--- a/C4/Templates.pm
>+++ b/C4/Templates.pm
>@@ -349,6 +349,7 @@ sub getlanguagecookie {
>         -value   => $language,
>         -HttpOnly => 1,
>         -expires => '+3y',
>+        -sameSite => 'Lax',
>         -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>     );
> 
>diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl
>index 1ad9edb503..b7508db1a7 100755
>--- a/cataloguing/additem.pl
>+++ b/cataloguing/additem.pl
>@@ -220,7 +220,8 @@ if ($op eq "additem") {
>                     # We encode_base64url the whole freezed structure so we're sure we won't have any encoding problems
>                     -value   => encode_base64url( freeze( { %{$item->unblessed}, itemnumber => undef } ) ),
>                     -HttpOnly => 1,
>-                    -expires => ''
>+                    -expires => '',
>+                    -sameSite => 'Lax'
>                 );
> 
>                 $cookie = [ $cookie, $last_created_item_cookie ];
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/authorities_js.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/authorities_js.inc
>index a7d277ef84..6666c5b03d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/authorities_js.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/authorities_js.inc
>@@ -12,7 +12,7 @@ function mergeAuth(authid, summary) {
>         }
>         window.location.href = "/cgi-bin/koha/authorities/merge.pl?authid=" + authid + "&authid=" + alreadySelected.authid + refstring;
>     } else {
>-        Cookies.set('auth_to_merge', JSON.stringify({ 'authid': authid, 'summary': summary }), { 'path' : '/' });
>+        Cookies.set('auth_to_merge', JSON.stringify({ 'authid': authid, 'summary': summary }), { 'path' : '/', sameSite: 'Lax' });
>         showMergingInProgress();
>     }
> }
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
>index 352e9a0c53..9fd33640e8 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
>@@ -1207,7 +1207,7 @@ require( [ 'koha-backend', 'search', 'macros', 'marc-editor', 'marc-record', 'pr
>         $( '#switch-editor' ).click( function() {
>             if ( !confirm( _("Any changes will not be saved. Continue?") ) ) return;
> 
>-            Cookies.set( "catalogue_editor_[% logged_in_user.borrowernumber | html %]", "basic", { expires: 365, path: '/' } );
>+            Cookies.set( "catalogue_editor_[% logged_in_user.borrowernumber | html %]", "basic", { expires: 365, path: '/', sameSite: 'Lax'} );
> 
>             if ( state.backend == 'catalog' ) {
>                 window.location = '/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=' + state.recordID;
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
>index 88b58d9446..f28fa343ac 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
>@@ -329,7 +329,7 @@ MARC frameworks &rsaquo; Administration &rsaquo; Koha
>             }));
>             $("#select_display").on("change",function(){
>                 var checked = $(this).prop("checked") ? 1: 0;
>-                Cookies.set("marctagstructure_selectdisplay", checked);
>+                Cookies.set("marctagstructure_selectdisplay", checked, { sameSite: 'Lax' });
>                 this.form.submit();
>             });
>         });
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>index 0f9c02a233..00ea9c73f7 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>@@ -130,7 +130,7 @@
> 
>             if ( !confirm( breedingid ? _("This record cannot be transferred to the advanced editor. Continue?") : _("Any changes will not be saved. Continue?") ) ) return false;
> 
>-            Cookies.set( 'catalogue_editor_[% logged_in_user.borrowernumber | html %]', 'advanced', { expires: 365, path: '/' } );
>+            Cookies.set( 'catalogue_editor_[% logged_in_user.borrowernumber | html %]', 'advanced', { expires: 365, path: '/', sameSite: 'Lax'  } );
> 
>             var biblionumber = [% biblionumber || "null" | html %];
> 
>@@ -508,11 +508,11 @@ function PopupMARCFieldDoc(field) {
>     function toggleMARCdocLinks(flag){
>         if( flag === true ){
>             $(".marcdocs").show();
>-            Cookies.set("marcdocs_[% borrowernumber | html %]",'show', { path: "/", expires: 365 });
>+            Cookies.set("marcdocs_[% borrowernumber | html %]",'show', { path: "/", expires: 365, sameSite: 'Lax'  });
>             $("#marcDocsSelect i").addClass('fa-check-square-o').removeClass('fa-square-o');
>         } else {
>             $(".marcdocs").hide();
>-            Cookies.set("marcdocs_[% borrowernumber | html %]",'hide', { path: "/", expires: 365 });
>+            Cookies.set("marcdocs_[% borrowernumber | html %]",'hide', { path: "/", expires: 365, sameSite: 'Lax'  });
>             $("#marcDocsSelect i").removeClass('fa-check-square-o').addClass('fa-square-o');
>         }
>     }
>@@ -521,12 +521,12 @@ function PopupMARCFieldDoc(field) {
>         if( flag === true ){
>             $(".tagnum").show();
>             $(".subfieldcode").show();
>-            Cookies.set("marctags_[% borrowernumber | html %]",'show', { path: "/", expires: 365 });
>+            Cookies.set("marctags_[% borrowernumber | html %]",'show', { path: "/", expires: 365, sameSite: 'Lax'  });
>             $("#marcTagsSelect i").addClass('fa-check-square-o').removeClass('fa-square-o');
>         } else {
>             $(".tagnum").hide();
>             $(".subfieldcode").hide();
>-            Cookies.set("marctags_[% borrowernumber | html %]",'hide', { path: "/", expires: 365 });
>+            Cookies.set("marctags_[% borrowernumber | html %]",'hide', { path: "/", expires: 365, sameSite: 'Lax'  });
>             $("#marcTagsSelect i").removeClass('fa-check-square-o').addClass('fa-square-o');
>         }
>     }
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbooks.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbooks.tt
>index 74f9bd7e86..267ce5ceb4 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbooks.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbooks.tt
>@@ -295,7 +295,7 @@
>             });
> 
>             $("#useadvanced").click(function(){
>-                Cookies.set( "catalogue_editor_[% logged_in_user.borrowernumber | html %]", "advanced", { expires: 365, path: '/' } );
>+                Cookies.set( "catalogue_editor_[% logged_in_user.borrowernumber | html %]", "advanced", { expires: 365, path: '/', sameSite: 'Lax' } );
>                 return true;
>             });
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt
>index ebf7339bd2..b55a054fc4 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt
>@@ -235,7 +235,7 @@
>         function SearchToHold(club_id) {
>             var date = new Date();
>             date.setTime(date.getTime() + (10 * 60 * 1000));
>-            Cookies.set("holdforclub", club_id, { path: "/", expires: date });
>+            Cookies.set("holdforclub", club_id, { path: "/", expires: date, sameSite: 'Lax'  });
>             location.href="/cgi-bin/koha/catalogue/search.pl";
>         }
>     </script>
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/acquisitions-menu.js b/koha-tmpl/intranet-tmpl/prog/js/acquisitions-menu.js
>index 97597dd4d9..313ae0f5ef 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/acquisitions-menu.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/acquisitions-menu.js
>@@ -3,7 +3,7 @@ function searchToOrder( basketno, vendorid ){
>     var cookieData = "";
>     date.setTime(date.getTime() + (10 * 60 * 1000));
>     cookieData += basketno + "/" + vendorid;
>-    Cookies.set("searchToOrder", cookieData, { path: "/", expires: date });
>+    Cookies.set("searchToOrder", cookieData, { path: "/", expires: date, sameSite: 'Lax'  });
> }
> 
> $(document).ready(function() {
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>index 2718fec83e..88c59396d1 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>@@ -258,7 +258,7 @@ $(document).ready(function() {
>         $('#issues-table-load-immediately').prop('checked', true);
>     }
>     $('#issues-table-load-immediately').on( "change", function(){
>-        Cookies.set("issues-table-load-immediately-" + script, $(this).is(':checked'), { expires: 365 });
>+        Cookies.set("issues-table-load-immediately-" + script, $(this).is(':checked'), { expires: 365, sameSite: 'Lax'  });
>     });
> 
>     function RefreshIssuesTable() {
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/members-menu.js b/koha-tmpl/intranet-tmpl/prog/js/members-menu.js
>index 1b4f31c48b..7b7e6ff1c1 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/members-menu.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/members-menu.js
>@@ -168,6 +168,6 @@ function printx_window(print_type) {
> function searchToHold(){
>     var date = new Date();
>     date.setTime(date.getTime() + (10 * 60 * 1000));
>-    Cookies.set("holdfor", borrowernumber, { path: "/", expires: date });
>+    Cookies.set("holdfor", borrowernumber, { path: "/", expires: date, sameSite: 'Lax'  });
>     location.href="/cgi-bin/koha/catalogue/search.pl";
> }
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/pages/batchMod.js b/koha-tmpl/intranet-tmpl/prog/js/pages/batchMod.js
>index e78ed98ac8..987bd3537b 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/pages/batchMod.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/pages/batchMod.js
>@@ -52,10 +52,10 @@ function hideColumn(num) {
>         if (!found) {
>             valCookie.push(hide);
>             var cookieString = valCookie.join("/");
>-            Cookies.set("showColumns", cookieString, { expires: date, path: '/' });
>+            Cookies.set("showColumns", cookieString, { expires: date, path: '/', sameSite: 'Lax'  });
>         }
>     } else {
>-        Cookies.set("showColumns", hide, { expires: date, path: '/' });
>+        Cookies.set("showColumns", hide, { expires: date, path: '/', sameSite: 'Lax'  });
>     }
> }
> 
>@@ -88,7 +88,7 @@ function showColumn(num) {
>         }
>         if (found) {
>             var cookieString = valCookie.join("/");
>-            Cookies.set("showColumns", cookieString, { expires: date, path: '/' });
>+            Cookies.set("showColumns", cookieString, { expires: date, path: '/', sameSite: 'Lax'  });
>         }
>     }
> }
>@@ -113,7 +113,7 @@ function hideAllColumns() {
>     $("#itemst td:nth-child("+nb_cols+"),#itemst tr th:nth-child("+nb_cols+")").nextAll().hide();
>     $("#hideall").prop("checked", true).parent().addClass("selected");
>     var cookieString = allColumns.join("/");
>-    Cookies.set("showColumns", cookieString, { expires: date, path: '/' });
>+    Cookies.set("showColumns", cookieString, { expires: date, path: '/', sameSite: 'Lax'  });
> }
> 
> $(document).ready(function () {
>diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl
>index b293f100e1..a89f2538d6 100755
>--- a/opac/sco/sco-main.pl
>+++ b/opac/sco/sco-main.pl
>@@ -376,6 +376,7 @@ $cookie = $query->cookie(
>     -expires => $jwt ? '+1d' : '',
>     -HttpOnly => 1,
>     -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
>+    -sameSite => 'Lax'
> );
> 
> $template->param(patronid => $patronid);
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 26019: 130565 | 130566 | 131836 | 131837 | 132804 | 132805 | 133212 | 133213 | 133214