From 2fae8292cd2307a9276cfe899a13d24c46d3d8bc Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Tue, 23 Jun 2015 12:17:04 +0300 Subject: [PATCH] Bug 14437 - Refactor C4::Auth::haspermission() to Koha::Object and return better errors. Refactoring haspermission() and improving documentation. Also added a return value unintrusively (in the old context still RETURNS the same result) to tell which permission failed. Modification should have no adverse effects, other than killing the process (by throwing uncaught exceptions) if no valid parameters are given. Depends on Buugg 13995 - Proper Exception handling so apply it first. Cannot create unit tests because there is no CRUD interface for permissions. Not even with TestBuilder. TEST PLAN: 1. Find a borrower with only the catalogue-permission. 2. try opening this page cgi-bin/koha/tools/manage-marc-import.pl 3. You get "Permission denied" --- C4/Auth.pm | 150 +++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 124 insertions(+), 26 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 34c3f91..885542e 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -23,6 +23,7 @@ use Digest::MD5 qw(md5_base64); use JSON qw/encode_json/; use URI::Escape; use CGI::Session; +use Scalar::Util qw(blessed); require Exporter; use C4::Context; @@ -31,11 +32,15 @@ use C4::Languages; use C4::Branch; # GetBranches use C4::Search::History; use Koha; +use Koha::Borrowers; use Koha::AuthUtils qw(hash_password); use POSIX qw/strftime/; use List::MoreUtils qw/ any /; use Encode qw( encode is_utf8); +use Koha::Exception::BadParameter; +use Koha::Exception::UnknownObject; + # use utf8; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $ldap $cas $caslogout $shib $shib_login); @@ -1917,22 +1922,82 @@ sub get_all_subpermissions { } =head2 haspermission +PUBLIC + + my ($failedPermission, $borrowerPermissions) = C4::Auth::haspermission($borrowerOrIdentifier, $flagsrequired); + #or + #If you don't care about the reason why the permission check failed + my $borrowerPermissions = C4::Auth::haspermission($borrowerOrIdentifier, $flagsrequired); + #or + #Just get the borrower permissions + my $borrowerPermissions = C4::Auth::haspermission($borrowerOrIdentifier); + +Finds the borrower's permission flags or checks if the borrower has the proper +permissions if $flagsrequired is given. + +C<$borrowerOrIdentifier> MANDATORY, either the userid of the borrower + or a Koha::Borrower-object +C<$flagsrequired> OPTIONAL, is a hashref of required flags + { borrowers => 1, #Must have all borrowers permission + circulate => *, #Must have any circulate permission + tools => 'stage_marc_import', #Must have the specific permission + } - $flags = ($userid, $flagsrequired); +C A List of String and a Reference to a HASH of HASHes, member's flags. Eg. + C + ( + undef, ##No failed permission + { tools => 1, ##borrower permissions + catalogue => 1, #All catalogue permissions + circulate => { override_renewals => 1, + force_checkout => 1, + }, + lists => 0, #No lists permissions + ... + } + ) + C + ( + "circulate => force_checkout", ##The failed permission, see. C4::Auth::check_permissions() + 0 ##Notify that the check failed. + ) + +C Koha::Exception::UnknownObject, if no borrower is found. + Koha::Exception::BadParameter, if parameters are not properly given. +=cut -C<$userid> the userid of the member -C<$flags> is a hashref of required flags like C<$borrower-<{authflags}> +sub haspermission { + my ( $borrOrId, $flagsrequired ) = @_; + my ($borrower); -Returns member's flags or 0 if a permission is not met. -=cut + ##Validate parameters since this is a public subroutine + #The best thing would be to get a honest reference to the Borrower-object, + #but currently we can only prepare that some day somebody will use me properly. + if (blessed($borrOrId) && $borrOrId->isa('Koha::Borrower')) { + $borrower = $borrOrId; + } + #We can get back to the DBIx to refetch the Borrower-object from the given borrower identifier, again. + elsif ($borrOrId && length($borrOrId) > 0) { + my @b = Koha::Borrowers->search( {userid => $borrOrId} ); + if ( $borrOrId eq C4::Context->config('user') ) { #Mock a Borrower-object for superuser + $borrower = Koha::Borrower->new(); + $borrower->update({userid => $borrOrId}); + } + elsif (scalar(@b)) { + $borrower = $b[0]; + } + else { + Koha::Exception::UnknownObject->throw(error => __PACKAGE__."::haspermission():> No borrower found with the given borrower identifier '$borrOrId'."); + } + } + else { + Koha::Exception::BadParameter->throw(error => __PACKAGE__."::haspermission():> No borrower or borrowerIdentifier given."); + } -sub haspermission { - my ( $userid, $flagsrequired ) = @_; - my $sth = C4::Context->dbh->prepare("SELECT flags FROM borrowers WHERE userid=?"); - $sth->execute($userid); - my $row = $sth->fetchrow(); - my $flags = getuserflags( $row, $userid ); + ##Parameters validated, time to get the flags! + my $userid = $borrower->userid(); + my $flags = getuserflags( $borrower->flags(), $userid ); if ( $userid eq C4::Context->config('user') ) { # Super User Account from /etc/koha.conf @@ -1944,26 +2009,59 @@ sub haspermission { $flags->{'superlibrarian'} = 1; } - return $flags if $flags->{superlibrarian}; + #This fcn returns the failed permission so a suitable error msg can be delivered. + my $failedPermission = ($flagsrequired) ? check_permissions($flagsrequired, $flags) : undef; + if ($failedPermission) { + return ($failedPermission, 0); + } + return ($failedPermission, $flags); +} + +=head check_permissions +PUBLIC + + my $failedPermission = check_permissions($permissionsRequired, $permissions); + +Check if the given $permissions satisfy the $permissionsRequired. +@PARAM1 Reference to HASH of HASHes, see. C4::Auth::haspermission() $flagsrequired +@PARAM2 Reference to HASH of HASHes, see. C4::Auth::haspermission() @RETURNS +@RETURNS String, the failed permission, eg: + "circulate == 1", or "circulate == *", or "circulate => force_checkout" + or + undef if everything went ok. +@THROWS Koha::Exception::BadParameter, if all parameters are not properly given. +=cut + +sub check_permissions { + my ($permissionsRequired, $permissions) = @_; + + ##Validate parameters, since this is a public subroutine + unless ($permissionsRequired && ref($permissionsRequired) eq 'HASH' && scalar(%$permissionsRequired)) { + Koha::Exception::BadParameter->throw(error => __PACKAGE__."::check_permission():> Mandatory parameter \$permissionsRequired is not given or is not a reference to HASH, or the HASH is empty."); + } + unless ($permissions && ref($permissions) eq 'HASH' && scalar(%$permissions)) { + Koha::Exception::BadParameter->throw(error => __PACKAGE__."::check_permission():> Mandatory parameter \$permissions is not given or is not a reference to HASH, or the HASH is empty."); + } + + ##Start checking permissions + return undef if $permissions->{superlibrarian}; - foreach my $module ( keys %$flagsrequired ) { - my $subperm = $flagsrequired->{$module}; + my $failedPermission; + foreach my $module ( keys %$permissionsRequired ) { + my $subperm = $permissionsRequired->{$module}; if ( $subperm eq '*' ) { - return 0 unless ( $flags->{$module} == 1 or ref( $flags->{$module} ) ); + $failedPermission = {$module => '*'} unless ( $permissions->{$module} == 1 or + ref( $permissions->{$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 ) - ); + $failedPermission = {$module => '1'} unless ( defined($permissions->{$module}) and + $permissions->{$module} == 1 ); + $failedPermission = {$module => $subperm} unless ( ref($permissions->{$module}) and + exists($permissions->{$module}->{$subperm}) and + $permissions->{$module}->{$subperm} == 1 ); } + last() if $failedPermission; } - return $flags; - - #FIXME - This fcn should return the failed permission so a suitable error msg can be delivered. + return $failedPermission || undef; } sub getborrowernumber { -- 1.7.9.5