Attachment 86197 Details for Bug 22031 – Bug 22031: Add SQL::Abstract like syntax to haspermission

[patch] Bug 22031: Add SQL::Abstract like syntax to haspermission

Bug-22031-Add-SQLAbstract-like-syntax-to-haspermis.patch (text/plain), 13.46 KB, created by Nick Clemens (kidclamp) on 2019-03-06 17:13:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2019-03-06 17:13:45 UTC

Size: 13.46 KB

patch

obsolete

>From 9178951ff74f4a4f70c834a53ce25dbcb6d7c221 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Sat, 22 Dec 2018 13:55:23 +0000
>Subject: [PATCH] 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>
>---
> C4/Auth.pm                          |  59 ++++++---
> t/db_dependent/Auth/haspermission.t | 248 +++++++++++++++++++++++++-----------
> 2 files changed, 214 insertions(+), 93 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index b19197b35b..8932066d48 100644
>--- 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 bf0290d6be..672832b9ee 100644
>--- 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;
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 22031: 83414 | 83415 | 83474 | 86197 | 86204