Bug 22031: Add SQL::Abstract like syntax to haspermission

This patch adds an SQL::Abstract inspired query syntax to the
haspermission method in C4::Auth.  One can now pass Arrayrefs to denote
an OR list of flags, a Hashref to denote a AND list of flags.

Structures can be nested at arbitrary depth.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index b19197b..8932066 100644 (file)
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -19,6 +19,8 @@ package C4::Auth;
 
 use strict;
 use warnings;
+use Carp qw/croak/;
+
 use Digest::MD5 qw(md5_base64);
 use JSON qw/encode_json/;
 use URI::Escape;
@@ -2025,12 +2027,49 @@ sub get_all_subpermissions {
   $flags = ($userid, $flagsrequired);
 
 C<$userid> the userid of the member
-C<$flags> is a hashref of required flags like C<$borrower-&lt;{authflags}> 
+C<$flags> is a query structure similar to that used by SQL::Abstract that
+denotes the combination of flags required.
+
+The main logic of this method is that things in arrays are OR'ed, and things
+in hashes are AND'ed.
 
 Returns member's flags or 0 if a permission is not met.
 
 =cut
 
+sub _dispatch {
+    my ($required, $flags) = @_;
+
+    my $ref = ref($required);
+    if ($ref eq '') {
+        if ($required eq '*') {
+            return 0 unless ( $flags or ref( $flags ) );
+        } else {
+            return 0 unless ( $flags and (!ref( $flags ) || $flags->{$required} ));
+        }
+    } elsif ($ref eq 'HASH') {
+        foreach my $key (keys %{$required}) {
+            my $require = $required->{$key};
+            my $rflags  = $flags->{$key};
+            return 0 unless _dispatch($require, $rflags);
+        }
+    } elsif ($ref eq 'ARRAY') {
+        my $satisfied = 0;
+        foreach my $require ( @{$required} ) {
+            my $rflags =
+              ( ref($flags) && !ref($require) && ( $require ne '*' ) )
+              ? $flags->{$require}
+              : $flags;
+            $satisfied++ if _dispatch( $require, $rflags );
+        }
+        return 0 unless $satisfied;
+    } else {
+        croak "Unexpected structure found: $ref";
+    }
+
+    return $flags;
+};
+
 sub haspermission {
     my ( $userid, $flagsrequired ) = @_;
     my $sth = C4::Context->dbh->prepare("SELECT flags FROM borrowers WHERE userid=?");
@@ -2039,23 +2078,7 @@ sub haspermission {
     my $flags = getuserflags( $row, $userid );
 
     return $flags if $flags->{superlibrarian};
-
-    foreach my $module ( keys %$flagsrequired ) {
-        my $subperm = $flagsrequired->{$module};
-        if ( $subperm eq '*' ) {
-            return 0 unless ( $flags->{$module} == 1 or ref( $flags->{$module} ) );
-        } else {
-            return 0 unless (
-                ( defined $flags->{$module} and
-                    $flags->{$module} == 1 )
-                or
-                ( ref( $flags->{$module} ) and
-                    exists $flags->{$module}->{$subperm} and
-                    $flags->{$module}->{$subperm} == 1 )
-            );
-        }
-    }
-    return $flags;
+    return _dispatch($flagsrequired, $flags);
 
     #FIXME - This fcn should return the failed permission so a suitable error msg can be delivered.
 }

diff --git a/t/db_dependent/Auth/haspermission.t b/t/db_dependent/Auth/haspermission.t
index bf0290d..672832b 100644 (file)
--- 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 => 13;
+use Test::More tests => 3;
 
 use Koha::Database;
 use t::lib::TestBuilder;
@@ -31,78 +31,176 @@ $schema->storage->txn_begin;
 
 # Adding two borrowers and granular permissions for the second borrower
 my $builder = t::lib::TestBuilder->new();
-my $borr1 = $builder->build({
-    source => 'Borrower',
-    value  => {
-        surname => 'Superlib',
-        flags   => 1,
-    },
-});
-my $borr2 = $builder->build({
-    source => 'Borrower',
-    value  => {
-        surname => 'Bor2',
-        flags   => 2 + 4 + 2**11, # circulate, catalogue, acquisition
-    },
-});
-$builder->build({
-    source => 'UserPermission',
-    value  => {
-        borrowernumber => $borr2->{borrowernumber},
-        module_bit => 13, # tools
-        code => 'upload_local_cover_images',
-    },
-});
-$builder->build({
-    source => 'UserPermission',
-    value  => {
-        borrowernumber => $borr2->{borrowernumber},
-        module_bit => 13, # tools
-        code => 'batch_upload_patron_images',
-    },
-});
-
-# Check top level permission for superlibrarian
-my $r = haspermission( $borr1->{userid},
-    { circulate => 1, editcatalogue => 1 } );
-is( ref($r), 'HASH', 'Superlibrarian/circulate' );
-
-# Check specific top level permission(s) for borr2
-$r = haspermission( $borr2->{userid},
-    { circulate => 1, catalogue => 1 } );
-is( ref($r), 'HASH', 'Borrower2/circulate' );
-$r = haspermission( $borr2->{userid}, { updatecharges => 1 } );
-is( $r, 0, 'Borrower2/updatecharges should fail' );
-
-# Check granular permission with 1: means all subpermissions
-$r = haspermission( $borr1->{userid}, { tools => 1 } );
-is( ref($r), 'HASH', 'Superlibrarian/tools granular all' );
-$r = haspermission( $borr2->{userid}, { tools => 1 } );
-is( $r, 0, 'Borrower2/tools granular all should fail' );
-
-# Check granular permission with *: means at least one subpermission
-$r = haspermission( $borr1->{userid}, { tools => '*' } );
-is( ref($r), 'HASH', 'Superlibrarian/tools granular *' );
-$r = haspermission( $borr2->{userid}, { acquisition => '*' } );
-is( ref($r), 'HASH', 'Borrower2/acq granular *' );
-$r = haspermission( $borr2->{userid}, { tools => '*' } );
-is( ref($r), 'HASH', 'Borrower2/tools granular *' );
-$r = haspermission( $borr2->{userid}, { serials => '*' } );
-is( $r, 0, 'Borrower2/serials granular * should fail' );
-
-# Check granular permission with one or more specific subperms
-$r = haspermission( $borr1->{userid}, { tools => 'edit_news' } );
-is( ref($r), 'HASH', 'Superlibrarian/tools edit_news' );
-$r = haspermission( $borr2->{userid}, { acquisition => 'budget_manage' } );
-is( ref($r), 'HASH', 'Borrower2/acq budget_manage' );
-$r = haspermission( $borr2->{userid},
-    { acquisition => 'budget_manage', tools => 'edit_news' } );
-is( $r, 0, 'Borrower2/two granular should fail' );
-$r = haspermission( $borr2->{userid}, {
-    tools => 'upload_local_cover_images',
-    tools => 'batch_upload_patron_images',
-});
-is( ref($r), 'HASH', 'Borrower2/tools granular two upload subperms' );
-
-# End
+my $borr1   = $builder->build(
+    {
+        source => 'Borrower',
+        value  => {
+            surname => 'Superlib',
+            flags   => 1,
+        },
+    }
+);
+my $borr2 = $builder->build(
+    {
+        source => 'Borrower',
+        value  => {
+            surname => 'Bor2',
+            flags   => 2 + 4 + 2**11,    # circulate, catalogue, acquisition
+        },
+    }
+);
+$builder->build(
+    {
+        source => 'UserPermission',
+        value  => {
+            borrowernumber => $borr2->{borrowernumber},
+            module_bit     => 13,                            # tools
+            code           => 'upload_local_cover_images',
+        },
+    }
+);
+$builder->build(
+    {
+        source => 'UserPermission',
+        value  => {
+            borrowernumber => $borr2->{borrowernumber},
+            module_bit     => 13,                             # tools
+            code           => 'batch_upload_patron_images',
+        },
+    }
+);
+
+subtest 'scalar top level tests' => sub {
+
+    plan tests => 3;
+
+    # Check top level permission for superlibrarian
+    my $r = haspermission( $borr1->{userid}, 'circulate' );
+    is( ref($r), 'HASH', 'Superlibrarian/circulate' );
+
+    # Check specific top level permission(s) for borr2
+    $r = haspermission( $borr2->{userid}, 'circulate' );
+    is( ref($r), 'HASH', 'Borrower2/circulate' );
+    $r = haspermission( $borr2->{userid}, 'updatecharges' );
+    is( $r, 0, 'Borrower2/updatecharges should fail' );
+};
+
+subtest 'hashref top level AND tests' => sub {
+
+    plan tests => 15;
+
+    # Check top level permission for superlibrarian
+    my $r =
+      haspermission( $borr1->{userid}, { circulate => 1 } );
+    is( ref($r), 'HASH', 'Superlibrarian/circulate' );
+
+    # Check specific top level permission(s) for borr2
+    $r = haspermission( $borr2->{userid}, { circulate => 1, catalogue => 1 } );
+    is( ref($r), 'HASH', 'Borrower2/circulate' );
+    $r = haspermission( $borr2->{userid}, { updatecharges => 1 } );
+    is( $r, 0, 'Borrower2/updatecharges should fail' );
+
+    # Check granular permission with 1: means all subpermissions
+    $r = haspermission( $borr1->{userid}, { tools => 1 } );
+    is( ref($r), 'HASH', 'Superlibrarian/tools granular all' );
+    $r = haspermission( $borr2->{userid}, { tools => 1 } );
+    is( $r, 0, 'Borrower2/tools granular all should fail' );
+
+    # Check granular permission with *: means at least one subpermission
+    $r = haspermission( $borr1->{userid}, { tools => '*' } );
+    is( ref($r), 'HASH', 'Superlibrarian/tools granular *' );
+    $r = haspermission( $borr2->{userid}, { acquisition => '*' } );
+    is( ref($r), 'HASH', 'Borrower2/acq granular *' );
+    $r = haspermission( $borr2->{userid}, { tools => '*' } );
+    is( ref($r), 'HASH', 'Borrower2/tools granular *' );
+    $r = haspermission( $borr2->{userid}, { serials => '*' } );
+    is( $r, 0, 'Borrower2/serials granular * should fail' );
+
+    # Check granular permission with one or more specific subperms
+    $r = haspermission( $borr1->{userid}, { tools => 'edit_news' } );
+    is( ref($r), 'HASH', 'Superlibrarian/tools edit_news' );
+    $r = haspermission( $borr2->{userid}, { acquisition => 'budget_manage' } );
+    is( ref($r), 'HASH', 'Borrower2/acq budget_manage' );
+    $r = haspermission( $borr2->{userid},
+        { acquisition => 'budget_manage', tools => 'edit_news' } );
+    is( $r, 0, 'Borrower2 (/acquisition|budget_manage AND /tools|edit_news) should fail' );
+    $r = haspermission(
+        $borr2->{userid},
+        {
+            tools => {
+                'upload_local_cover_images'  => 1,
+                'batch_upload_patron_images' => 1
+            },
+        }
+    );
+    is( ref($r), 'HASH', 'Borrower2 (/tools|upload_local_cover_image AND /tools|batch_upload_patron_images) granular' );
+    $r = haspermission(
+        $borr2->{userid},
+        {
+            tools => {
+                'upload_local_cover_images'  => 1,
+                'edit_news' => 1
+            },
+        }
+    );
+    is( $r, 0, 'Borrower2 (/tools|upload_local_cover_image AND /tools|edit_news) granular' );
+    $r = haspermission(
+        $borr2->{userid},
+        {
+            tools => [ 'upload_local_cover_images', 'edit_news'],
+        }
+    );
+    is( ref($r), 'HASH', 'Borrower2 (/tools|upload_local_cover_image OR /tools|edit_news) granular' );
+};
+
+subtest 'arrayref top level OR tests' => sub {
+
+    plan tests => 13;
+
+    # Check top level permission for superlibrarian
+    my $r =
+      haspermission( $borr1->{userid}, [ 'circulate', 'editcatalogue' ] );
+    is( ref($r), 'HASH', 'Superlibrarian/circulate' );
+
+    # Check specific top level permission(s) for borr2
+    $r = haspermission( $borr2->{userid}, [ 'circulate', 'updatecharges' ] );
+    is( ref($r), 'HASH', 'Borrower2/circulate OR Borrower2/updatecharges' );
+    $r = haspermission( $borr2->{userid}, ['updatecharges', 'serials' ] );
+    is( $r, 0, 'Borrower2/updatecharges OR Borrower2/serials should fail' );
+
+    # Check granular permission with 1: means all subpermissions
+    $r = haspermission( $borr1->{userid}, [ 'tools' ] );
+    is( ref($r), 'HASH', 'Superlibrarian/tools granular all' );
+    $r = haspermission( $borr2->{userid}, [ 'tools' ] );
+    is( $r, 0, 'Borrower2/tools granular all should fail' );
+
+    # Check granular permission with *: means at least one subpermission
+    $r = haspermission( $borr1->{userid}, [ { tools => '*' } ] );
+    is( ref($r), 'HASH', 'Superlibrarian/tools granular *' );
+    $r = haspermission( $borr2->{userid}, [ { acquisition => '*' } ] );
+    is( ref($r), 'HASH', 'Borrower2/acq granular *' );
+    $r = haspermission( $borr2->{userid}, [ { tools => '*' } ] );
+    is( ref($r), 'HASH', 'Borrower2/tools granular *' );
+    $r = haspermission( $borr2->{userid}, [ { serials => '*' } ] );
+    is( $r, 0, 'Borrower2/serials granular * should fail' );
+
+    # Check granular permission with one or more specific subperms
+    $r = haspermission( $borr1->{userid}, [ { tools => 'edit_news' } ] );
+    is( ref($r), 'HASH', 'Superlibrarian/tools edit_news' );
+    $r =
+      haspermission( $borr2->{userid}, [ { acquisition => 'budget_manage' } ] );
+    is( ref($r), 'HASH', 'Borrower2/acq budget_manage' );
+    $r = haspermission( $borr2->{userid},
+        [ { acquisition => 'budget_manage'}, { tools => 'edit_news' } ] );
+    is( ref($r), 'HASH', 'Borrower2/two granular OR should pass' );
+    $r = haspermission(
+        $borr2->{userid},
+        [
+            { tools => ['upload_local_cover_images'] },
+            { tools => ['edit_news'] }
+        ]
+    );
+    is( ref($r), 'HASH', 'Borrower2/tools granular OR subperms' );
+};
+
 $schema->storage->txn_rollback;