From b9c1f1d12f61d7e27b9c202bd3439f70bc5d3f1b Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 8 Feb 2024 21:29:54 +0000
Subject: [PATCH] Bug 36056: Clarify subpermissions AND behavior

Working on bug 31791, I found myself wondering if our current recursive
code in C4::Auth::haspermission() would allow checking AND on
subpermissions.

As it is not documented in the POD or tested, I decided to write some
unit tests for it.

It turned out it was well supported, so I decided to submit the tests,
and a small tweak in the POD to reflect that.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Auth/haspermission.t
=> SUCCESS: Tests pass! The code supports AND on subpermissions!
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit aa1049fdd37d87b73c434dfdea11ba691e7095da)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 26ba5fba8be7eee3d931c2d6ef774860145780f0)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 C4/Auth.pm                          |  1 +
 t/db_dependent/Auth/haspermission.t | 45 ++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 0f065bd5cd..a12b92bb38 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -2232,6 +2232,7 @@ sub get_cataloguing_page_permissions {
   $flagsrequired = { 'a_flag => 1, 'b_flag' => 1 };     # a_flag AND b_flag must be satisfied
   $flagsrequired = { 'a_flag' => 'sub_a' };             # sub_a of a_flag must be satisfied
   $flagsrequired = { 'a_flag' => [ 'sub_a, 'sub_b' ] }; # sub_a OR sub_b of a_flag must be satisfied
+  $flagsrequired = { 'a_flag' => { 'sub_a' => 1, 'sub_b' => 1 } };    # sub_a AND sub_b of a_flag must be satisfied
 
   $flags = ($userid, $flagsrequired);
 
diff --git a/t/db_dependent/Auth/haspermission.t b/t/db_dependent/Auth/haspermission.t
index b487358461..6cbb044561 100755
--- a/t/db_dependent/Auth/haspermission.t
+++ b/t/db_dependent/Auth/haspermission.t
@@ -20,7 +20,7 @@
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
-use Test::More tests => 4;
+use Test::More tests => 5;
 use Test::Exception;
 
 use Koha::Database;
@@ -238,3 +238,46 @@ subtest 'arrayref top level OR tests' => sub {
 };
 
 $schema->storage->txn_rollback;
+
+subtest 'AND on subpermissions' => sub {
+
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } );
+
+    $builder->build(
+        {
+            source => 'UserPermission',
+            value  => {
+                borrowernumber => $patron->id,
+                module_bit     => 9,                  # editcatalogue
+                code           => 'edit_catalogue',
+            },
+        }
+    );
+
+    my $r = haspermission( $patron->userid, { editcatalogue => { edit_catalogue => 1, advanced_editor => 1 } } );
+    is( $r, 0, "The user only has 'edit_catalogue' permissions, 0 returned" );
+
+    $builder->build(
+        {
+            source => 'UserPermission',
+            value  => {
+                borrowernumber => $patron->id,
+                module_bit     => 9,                   # editcatalogue
+                code           => 'advanced_editor',
+            },
+        }
+    );
+
+    $r = haspermission( $patron->userid, { editcatalogue => { edit_catalogue => 1, advanced_editor => 1 } } );
+
+    ok(
+        $r->{editcatalogue}->{edit_catalogue} && $r->{editcatalogue}->{advanced_editor},
+        "The patron has 'edit_catalogue' and 'advanced_editor', both returned"
+    );
+
+    $schema->storage->txn_rollback;
+};
-- 
2.39.5