From 9573461904e4e51acdd991d791a80d618fcc0126 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 8 Mar 2019 12:24:17 +0000 Subject: [PATCH] Bug 22483: Explicitly ban 'undef' as a valid $flagsrequired Before bug 22031 the haspermission subroutine signature allowed for passing 'undef' to mean 'any permission' in $flagsrequired. This feels like a mistake and was only in practical use in two places in the codebase. This patch explicitly forbids this practice (`*` may be used to the same result and is more explicit in it's nature) and replaces the two instances of it's use. Test Plan 1. Before this patch, the API tests are all failing with authentication errors 2. After this patch the API tests should now all pass. 3. t/db_dependent/Auth/haspermission.t should continue to pass (with one addition subtest added herin) 3. /svc/members/search is not unit tested. Please check that patron searching still yields results in the UI after this patch. Signed-off-by: Martin Renvoize --- C4/Auth.pm | 4 ++++ Koha/REST/V1/Auth.pm | 2 +- svc/members/search | 2 +- t/db_dependent/Auth/haspermission.t | 14 +++++++++++++- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 8932066d48..c49076e205 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -2072,6 +2072,10 @@ sub _dispatch { sub haspermission { my ( $userid, $flagsrequired ) = @_; + + Koha::Exceptions::WrongParameter->throw('$flagsrequired should not be undef') + unless defined($flagsrequired); + my $sth = C4::Context->dbh->prepare("SELECT flags FROM borrowers WHERE userid=?"); $sth->execute($userid); my $row = $sth->fetchrow(); diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index 92cafc22f2..71ef60a277 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -189,7 +189,7 @@ sub authenticate_api_request { # Manually pass the remote_address to check_auth_cookie my $remote_addr = $c->tx->remote_address; my ($status, $sessionID) = check_cookie_auth( - $cookie, undef, + $cookie, '*', { remote_addr => $remote_addr }); if ($status eq "ok") { my $session = get_session($sessionID); diff --git a/svc/members/search b/svc/members/search index 86c7473c52..13869c4b04 100755 --- a/svc/members/search +++ b/svc/members/search @@ -91,7 +91,7 @@ if ($has_permission) { my ( $permission, $subpermission ) = split /\./, $has_permission; my @patrons_with_permission; for my $patron ( @{ $results->{patrons} } ) { - my $perms = haspermission( $patron->{userid} ); + my $perms = haspermission( $patron->{userid}, '*' ); if ( $perms->{superlibrarian} == 1 or $perms->{$permission} == 1 ) { diff --git a/t/db_dependent/Auth/haspermission.t b/t/db_dependent/Auth/haspermission.t index 672832b9ee..2cde29aa56 100644 --- a/t/db_dependent/Auth/haspermission.t +++ b/t/db_dependent/Auth/haspermission.t @@ -20,7 +20,8 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 4; +use Test::Exception; use Koha::Database; use t::lib::TestBuilder; @@ -70,6 +71,17 @@ $builder->build( } ); +subtest 'undef top level tests' => sub { + + plan tests => 2; + + throws_ok { my $r = haspermission( $borr1->{userid} ); } + 'Koha::Exceptions::WrongParameter', + 'Exception thrown when missing $requiredflags'; + throws_ok { my $r = haspermission( $borr1->{userid}, undef ); } + 'Koha::Exceptions::WrongParameter', 'Exception thrown when explicit undef'; +}; + subtest 'scalar top level tests' => sub { plan tests => 3; -- 2.20.1