Bugzilla – Attachment 75543 Details for
Bug 20813
Revamp user permissions system
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20813: Revamp user permissions system
Bug-20813-Revamp-user-permissions-system.patch (text/plain), 44.10 KB, created by
Kyle M Hall (khall)
on 2018-05-24 13:45:31 UTC
(
hide
)
Description:
Bug 20813: Revamp user permissions system
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2018-05-24 13:45:31 UTC
Size:
44.10 KB
patch
obsolete
>From 5cb032eebd279b2d9d82d03f42a860c8581962ec Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Thu, 31 Aug 2017 10:48:47 -0400 >Subject: [PATCH] Bug 20813: Revamp user permissions system > >Koha's current user permissions system is a bit convoluted and limited >due to it's legacy where the permissions system was a simple set of >flags. A second layer of sub-permissions were later added complication >and obfuscation to the system. > >We should revamp the permissions system to be a more modern system and >open the path to having unlimited nesting of sub-permissions instead of >shoe-horning sub-permissions into the limited system we have. > >Test Plan: >1) Apply this patch set >2) Run updatedatabase.pl >3) Restart all the things! >4) Koha should still enforce user permissions as before >--- > C4/Acquisition.pm | 2 +- > C4/Auth.pm | 111 ++++------ > C4/Budgets.pm | 3 +- > Koha/Patron.pm | 50 +++++ > Koha/Patron/Permission.pm | 52 +++++ > Koha/Patron/Permissions.pm | 56 +++++ > Koha/Permission.pm | 73 ++++++ > Koha/Permissions.pm | 56 +++++ > about.pl | 22 +- > .../prog/en/includes/members-toolbar.inc | 2 +- > .../intranet-tmpl/prog/en/includes/permissions.inc | 7 +- > .../prog/en/modules/members/member-flags.tt | 167 ++++++++------ > members/member-flags.pl | 246 ++++++++------------- > 13 files changed, 530 insertions(+), 317 deletions(-) > create mode 100644 Koha/Patron/Permission.pm > create mode 100644 Koha/Patron/Permissions.pm > create mode 100644 Koha/Permission.pm > create mode 100644 Koha/Permissions.pm > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index a257017dc7..118fa7b3f3 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -848,7 +848,7 @@ sub CanUserManageBasket { > my ($flags) = $sth->fetchrow_array; > $sth->finish; > >- $userflags = C4::Auth::getuserflags($flags, $borrower->{userid}, $dbh); >+ $userflags = C4::Auth::getuserflags( { userid => $borrower->{userid} }); > } > > unless ($userflags->{superlibrarian} >diff --git a/C4/Auth.pm b/C4/Auth.pm >index 7442b1dd78..64b1157000 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -36,6 +36,7 @@ use Koha::AuthUtils qw(get_script_name hash_password); > use Koha::Library::Groups; > use Koha::Libraries; > use Koha::Patrons; >+use Koha::Permissions; > use POSIX qw/strftime/; > use List::MoreUtils qw/ any /; > use Encode qw( encode is_utf8); >@@ -57,7 +58,6 @@ BEGIN { > @EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &checkpw_internal &checkpw_hash > &get_all_subpermissions &get_user_subpermissions > ); >- %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] ); > $ldap = C4::Context->config('useldapserver') || 0; > $cas = C4::Context->preference('casAuthentication'); > $shib = C4::Context->config('useshibboleth') || 0; >@@ -1065,7 +1065,7 @@ sub checkauth { > > if ( $return == 1 ) { > my $select = " >- SELECT borrowernumber, firstname, surname, flags, borrowers.branchcode, >+ SELECT borrowernumber, firstname, surname, borrowers.branchcode, > branches.branchname as branchname, > branches.branchprinter as branchprinter, > email >@@ -1088,10 +1088,10 @@ sub checkauth { > } > } > if ( $sth->rows ) { >- ( $borrowernumber, $firstname, $surname, $userflags, >+ ( $borrowernumber, $firstname, $surname, > $branchcode, $branchname, $branchprinter, $emailaddress ) = $sth->fetchrow; > $debug and print STDERR "AUTH_3 results: " . >- "$cardnumber,$borrowernumber,$userid,$firstname,$surname,$userflags,$branchcode,$emailaddress\n"; >+ "$cardnumber,$borrowernumber,$userid,$firstname,$surname,$branchcode,$emailaddress\n"; > } else { > print STDERR "AUTH_3: no results for userid='$userid', cardnumber='$cardnumber'.\n"; > } >@@ -1143,7 +1143,6 @@ sub checkauth { > $session->param( 'surname', $surname ); > $session->param( 'branch', $branchcode ); > $session->param( 'branchname', $branchname ); >- $session->param( 'flags', $userflags ); > $session->param( 'emailaddress', $emailaddress ); > $session->param( 'ip', $session->remote_addr() ); > $session->param( 'lasttime', time() ); >@@ -1848,35 +1847,35 @@ sub checkpw_internal { > > my $sth = > $dbh->prepare( >- "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?" >+ "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?" > ); > $sth->execute($userid); > if ( $sth->rows ) { > my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, >- $surname, $branchcode, $branchname, $flags ) >+ $surname, $branchcode, $branchname ) > = $sth->fetchrow; > > if ( checkpw_hash( $password, $stored_hash ) ) { > > C4::Context->set_userenv( "$borrowernumber", $userid, $cardnumber, >- $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; >+ $firstname, $surname, $branchcode, $branchname ) unless $no_set_userenv; > return 1, $cardnumber, $userid; > } > } > $sth = > $dbh->prepare( >- "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where cardnumber=?" >+ "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname from borrowers join branches on borrowers.branchcode=branches.branchcode where cardnumber=?" > ); > $sth->execute($userid); > if ( $sth->rows ) { > my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, >- $surname, $branchcode, $branchname, $flags ) >+ $surname, $branchcode, $branchname ) > = $sth->fetchrow; > > if ( checkpw_hash( $password, $stored_hash ) ) { > > C4::Context->set_userenv( $borrowernumber, $userid, $cardnumber, >- $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; >+ $firstname, $surname, $branchcode, $branchname ) unless $no_set_userenv; > return 1, $cardnumber, $userid; > } > } >@@ -1900,47 +1899,39 @@ sub checkpw_hash { > > =head2 getuserflags > >- my $authflags = getuserflags($flags, $userid, [$dbh]); >+ my $authflags = getuserflags($userid); > > Translates integer flags into permissions strings hash. > >-C<$flags> is the integer userflags value ( borrowers.userflags ) > C<$userid> is the members.userid, used for building subpermissions >-C<$authflags> is a hashref of permissions > > =cut > > sub getuserflags { >- my $flags = shift; >- my $userid = shift; >- my $dbh = @_ ? shift : C4::Context->dbh; >+ my $params = shift; >+ >+ my $userid = $params->{userid}; >+ my $force_subpermissions = $params->{force_subpermissions}; >+ >+ my $patron = Koha::Patrons->find( { userid => $userid } ); >+ return unless $patron; >+ > my $userflags; >- { >- # I don't want to do this, but if someone logs in as the database >- # user, it would be preferable not to spam them to death with >- # numeric warnings. So, we make $flags numeric. >- no warnings 'numeric'; >- $flags += 0; >- } >- my $sth = $dbh->prepare("SELECT bit, flag, defaulton FROM userflags"); >- $sth->execute; >+ my @permissions = Koha::Permissions->search( { parent => undef } ); >+ foreach my $p (@permissions) { >+ my $patron_has = $p->patron_has( $patron->id ); > >- while ( my ( $bit, $flag, $defaulton ) = $sth->fetchrow ) { >- if ( ( $flags & ( 2**$bit ) ) || $defaulton ) { >- $userflags->{$flag} = 1; >+ if ( $patron_has && !$force_subpermissions ) { >+ $userflags->{ $p->code() } = $p->patron_has( $patron->id ); > } > else { >- $userflags->{$flag} = 0; >+ my @subpermissions = $p->subpermissions; >+ foreach my $sp ( $p->subpermissions ) { >+ $userflags->{ $p->code } = $sp->patron_has( $patron->id ); >+ } > } > } > >- # get subpermissions and merge with top-level permissions >- my $user_subperms = get_user_subpermissions($userid); >- foreach my $module ( keys %$user_subperms ) { >- next if $userflags->{$module} == 1; # user already has permission for everything in this module >- $userflags->{$module} = $user_subperms->{$module}; >- } >- > return $userflags; > } > >@@ -1972,20 +1963,7 @@ necessary to check borrowers.flags. > sub get_user_subpermissions { > my $userid = shift; > >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare( "SELECT flag, user_permissions.code >- FROM user_permissions >- JOIN permissions USING (module_bit, code) >- JOIN userflags ON (module_bit = bit) >- JOIN borrowers USING (borrowernumber) >- WHERE userid = ?" ); >- $sth->execute($userid); >- >- my $user_perms = {}; >- while ( my $perm = $sth->fetchrow_hashref ) { >- $user_perms->{ $perm->{'flag'} }->{ $perm->{'code'} } = 1; >- } >- return $user_perms; >+ return getuserflags( { userid => $userid, force_subpermissions => 1 } ); > } > > =head2 get_all_subpermissions >@@ -1993,25 +1971,22 @@ sub get_user_subpermissions { > my $perm_hashref = get_all_subpermissions(); > > Returns a hashref of hashrefs defining all specific >-permissions currently defined. The return value >-has the same structure as that of C<get_user_subpermissions>, >-except that the innermost hash value is the description >-of the subpermission. >+permissions currently defined. The return value has >+the same basic structure as that of C<get_user_subpermissions> > > =cut > > sub get_all_subpermissions { >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare( "SELECT flag, code >- FROM permissions >- JOIN userflags ON (module_bit = bit)" ); >- $sth->execute(); >- >- my $all_perms = {}; >- while ( my $perm = $sth->fetchrow_hashref ) { >- $all_perms->{ $perm->{'flag'} }->{ $perm->{'code'} } = 1; >+ my $permissions; >+ >+ my @permissions = Koha::Permissions->search({ parent => undef }); >+ foreach my $p ( @permissions ) { >+ foreach my $sp ( $p->subpermissions ) { >+ $permissions->{ $p->code }->{ $sp->code } = 1; >+ } > } >- return $all_perms; >+ >+ return $permissions; > } > > =head2 haspermission >@@ -2027,10 +2002,8 @@ Returns member's flags or 0 if a permission is not met. > > 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 ); >+ >+ my $flags = getuserflags( { userid => $userid } ); > if ( $userid eq C4::Context->config('user') ) { > > # Super User Account from /etc/koha.conf >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 644c5e59c9..c9c87de0b9 100644 >--- a/C4/Budgets.pm >+++ b/C4/Budgets.pm >@@ -971,8 +971,7 @@ sub CanUserUseBudget { > return 0 unless ($borrower and $budget); > > if (not defined $userflags) { >- $userflags = C4::Auth::getuserflags($borrower->{flags}, >- $borrower->{userid}); >+ $userflags = C4::Auth::getuserflags( { userid => $borrower->{userid} } ); > } > > unless ($userflags->{superlibrarian} >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 35cfce4ca1..8fd2425edd 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -34,6 +34,7 @@ use Koha::Old::Checkouts; > use Koha::Patron::Categories; > use Koha::Patron::HouseboundProfile; > use Koha::Patron::HouseboundRole; >+use Koha::Patron::Permissions; > use Koha::Patron::Images; > use Koha::Patrons; > use Koha::Virtualshelves; >@@ -1034,6 +1035,55 @@ sub generate_userid { > > =head2 Internal methods > >+=head3 has_permission >+ >+my $bool = $patron->has_permission( $permission_code ); >+ >+=cut >+ >+sub has_permission { >+ my ( $self, $code ) = @_; >+ >+ return Koha::Patron::Permissions->find( >+ { >+ borrowernumber => $self->id, >+ code => $code, >+ } >+ ) ? 1 : 0; >+} >+ >+=head3 set_permissions >+ >+$patron->set_permissions( { permissions => \@permissions } ); >+ >+Given a list of permissions, set the patron to those and only those permissions >+ >+=cut >+ >+sub set_permissions { >+ my ( $self, $params ) = @_; >+ >+ my @permissions = @{ $params->{permissions} }; >+ >+ $self->_result->result_source->schema->txn_do( >+ sub { >+ Koha::Patron::Permissions->search( { borrowernumber => $self->id } ) >+ ->delete(); >+ >+ Koha::Patron::Permission->new( >+ { >+ borrowernumber => $self->id, >+ code => $_ >+ } >+ )->store() >+ for @permissions; >+ } >+ ); >+ >+ return $self; >+} >+ >+ > =head3 _type > > =cut >diff --git a/Koha/Patron/Permission.pm b/Koha/Patron/Permission.pm >new file mode 100644 >index 0000000000..04ef51e9ff >--- /dev/null >+++ b/Koha/Patron/Permission.pm >@@ -0,0 +1,52 @@ >+package Koha::Patron::Permission; >+ >+# Copyright ByWater Solutions 2017 >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it under the >+# terms of the GNU General Public License as published by the Free Software >+# Foundation; either version 3 of the License, or (at your option) any later >+# version. >+# >+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >+# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License along >+# with Koha; if not, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use Modern::Perl; >+ >+use Carp; >+ >+use base qw(Koha::Object); >+ >+=head1 NAME >+ >+Koha::Patron::Permission - Koha User Permission Object class >+ >+This class represents a single permission that a patron has >+ >+=head1 API >+ >+=head2 Class Methods >+ >+=cut >+ >+=head3 type >+ >+=cut >+ >+sub _type { >+ return 'UserPermission'; >+} >+ >+=head1 AUTHOR >+ >+Kyle M Hall <kyle@bywatersolutions.com> >+ >+=cut >+ >+1; >diff --git a/Koha/Patron/Permissions.pm b/Koha/Patron/Permissions.pm >new file mode 100644 >index 0000000000..e3aaa0538d >--- /dev/null >+++ b/Koha/Patron/Permissions.pm >@@ -0,0 +1,56 @@ >+package Koha::Patron::Permissions; >+ >+# Copyright ByWater Solutions 2017 >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it under the >+# terms of the GNU General Public License as published by the Free Software >+# Foundation; either version 3 of the License, or (at your option) any later >+# version. >+# >+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >+# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License along >+# with Koha; if not, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use Modern::Perl; >+ >+use Koha::Patron::Permission; >+ >+use base qw(Koha::Objects); >+ >+=head1 NAME >+ >+Koha::Patron::Permissions - Koha Patron Permissions Object class >+ >+This class represents a set of permissions a patron has >+ >+=head1 API >+ >+=head2 Class Methods >+ >+=cut >+ >+=head3 type >+ >+=cut >+ >+sub _type { >+ return 'UserPermission'; >+} >+ >+sub object_class { >+ return 'Koha::Patron::Permission'; >+} >+ >+=head1 AUTHOR >+ >+Kyle M Hall <kyle@bywatersolutions.com> >+ >+=cut >+ >+1; >diff --git a/Koha/Permission.pm b/Koha/Permission.pm >new file mode 100644 >index 0000000000..798702cea1 >--- /dev/null >+++ b/Koha/Permission.pm >@@ -0,0 +1,73 @@ >+package Koha::Permission; >+ >+# Copyright ByWater Solutions 2017 >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it under the >+# terms of the GNU General Public License as published by the Free Software >+# Foundation; either version 3 of the License, or (at your option) any later >+# version. >+# >+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >+# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License along >+# with Koha; if not, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use Modern::Perl; >+ >+use Carp; >+ >+use Koha::Patron::Permissions; >+ >+use base qw(Koha::Object); >+ >+=head1 NAME >+ >+Koha::Permission - Koha Permission Object class >+ >+=head1 API >+ >+=head2 Class Methods >+ >+=cut >+ >+=head3 patron_has >+ >+ Accepts a patron id, returns true if patron has this permission, false otherwise >+=cut >+ >+sub patron_has { >+ my ( $self, $patron ) = @_; >+ >+ return Koha::Patron::Permissions->find( { borrowernumber => $patron, code => $self->code } ) ? 1 : 0; >+} >+ >+=head3 subpermissions >+ >+=cut >+ >+sub subpermissions { >+ my ( $self ) = @_; >+ >+ return Koha::Permissions->search( { parent => $self->code } ); >+} >+ >+=head3 type >+ >+=cut >+ >+sub _type { >+ return 'Permission'; >+} >+ >+=head1 AUTHOR >+ >+Kyle M Hall <kyle@bywatersolutions.com> >+ >+=cut >+ >+1; >diff --git a/Koha/Permissions.pm b/Koha/Permissions.pm >new file mode 100644 >index 0000000000..2be4cfb324 >--- /dev/null >+++ b/Koha/Permissions.pm >@@ -0,0 +1,56 @@ >+package Koha::Permissions; >+ >+# Copyright ByWater Solutions 2017 >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it under the >+# terms of the GNU General Public License as published by the Free Software >+# Foundation; either version 3 of the License, or (at your option) any later >+# version. >+# >+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >+# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License along >+# with Koha; if not, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use Modern::Perl; >+ >+use Koha::Permission; >+ >+use base qw(Koha::Objects); >+ >+=head1 NAME >+ >+Koha::Permissions - Koha Permissions Object class >+ >+This object represents a type of permission a user may have. >+ >+=head1 API >+ >+=head2 Class Methods >+ >+=cut >+ >+=head3 type >+ >+=cut >+ >+sub _type { >+ return 'Permission'; >+} >+ >+sub object_class { >+ return 'Koha::Permission'; >+} >+ >+=head1 AUTHOR >+ >+Kyle M Hall <kyle@bywatersolutions.com> >+ >+=cut >+ >+1; >diff --git a/about.pl b/about.pl >index 70900dd2c4..630fc7fbdb 100755 >--- a/about.pl >+++ b/about.pl >@@ -43,6 +43,7 @@ use Koha::DateUtils qw(dt_from_string output_pref); > use Koha::Acquisition::Currencies; > use Koha::Patron::Categories; > use Koha::Patrons; >+use Koha::Patron::Permissions; > use Koha::Caches; > use Koha::Config::SysPrefs; > use Koha::Illrequest::Config; >@@ -414,21 +415,14 @@ if ( C4::Context->preference('WebBasedSelfCheck') > and C4::Context->preference('AutoSelfCheckAllowed') > ) { > my $userid = C4::Context->preference('AutoSelfCheckID'); >- my $all_permissions = C4::Auth::get_user_subpermissions( $userid ); >- my ( $has_self_checkout_perm, $has_other_permissions ); >- while ( my ( $module, $permissions ) = each %$all_permissions ) { >- if ( $module eq 'self_check' ) { >- while ( my ( $permission, $flag ) = each %$permissions ) { >- if ( $permission eq 'self_checkout_module' ) { >- $has_self_checkout_perm = 1; >- } else { >- $has_other_permissions = 1; >- } >- } >- } else { >- $has_other_permissions = 1; >+ my $patron = Koha::Patrons->find( { userid => $userid } ); >+ my $has_self_checkout_perm = $patron->has_permission('self_checkout'); >+ my $has_other_permissions = Koha::Patron::Permissions->search( >+ { >+ borrowernumber => $patron->id, >+ -not => { code => 'self_checkout' } > } >- } >+ )->count(); > $template->param( > AutoSelfCheckPatronDoesNotHaveSelfCheckPerm => not ( $has_self_checkout_perm ), > AutoSelfCheckPatronHasTooManyPerm => $has_other_permissions, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc >index 9b5524d4c1..b1af18ead3 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc >@@ -51,7 +51,7 @@ > <li class="disabled"><a data-toggle="tooltip" data-placement="left" title="You are not authorized to renew patrons" id="renewpatron" href="#">Renew patron</a></li> > [% END %] > [% IF ( CAN_user_permissions ) %] >- <li><a id="patronflags" href="/cgi-bin/koha/members/member-flags.pl?member=[% patron.borrowernumber %]">Set permissions</a></li> >+ <li><a id="patronflags" href="/cgi-bin/koha/members/member-flags.pl?borrowernumber=[% patron.borrowernumber %]">Set permissions</a></li> > [% ELSE %] > <li class="disabled"><a data-toggle="tooltip" data-placement="left" title="You are not authorized to set permissions" id="patronflags" href="#">Set permissions</a></li> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >index 017ffb918e..dbd4c24707 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >@@ -1,4 +1,4 @@ >-[%- BLOCK main_permissions -%] >+[%- BLOCK permissions -%] > [% SWITCH name %] > [%- CASE 'superlibrarian' -%]<span>Access to all librarian functions</span> > [%- CASE 'circulate' -%]<span>Check out and check in items</span> >@@ -22,11 +22,6 @@ > [%- CASE 'clubs' -%]<span>Patron clubs</span> > [%- CASE 'ill' -%]<span>Create and modify Interlibrary loan requests</span> > [%- CASE 'self_check' -%]<span>Self check modules</span> >- [%- END -%] >-[%- END -%] >- >-[%- BLOCK sub_permissions -%] >- [% SWITCH name %] > [%- CASE 'circulate_remaining_permissions' -%]<span>Remaining circulation permissions</span> > [%- CASE 'force_checkout' -%]<span>Force checkout if a limitation exists</span> > [%- CASE 'manage_restrictions' -%]<span>Manage restrictions for accounts</span> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-flags.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-flags.tt >index 4d40e0acce..6c8c260675 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-flags.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/member-flags.tt >@@ -6,6 +6,43 @@ > <title>Koha › Patrons › Set permissions for [% patron.surname %], [% patron.firstname %]</title> > [% Asset.css("css/treeview/jquery.treeview.css") %] > [% INCLUDE 'doc-head-close.inc' %] >+<link href="[% interface %]/lib/jquery/plugins/treetable/stylesheets/jquery.treetable.css" rel="stylesheet" type="text/css" /> >+<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/treetable/jquery.treetable.js"></script> >+<script type="text/javascript"> >+ $(document).ready(function() { >+ $('#permissions-table').treetable({ >+ expandable: true >+ }); >+ >+ // Expand any parent permissions with children that are set >+ $('input[name="permission"][checked="checked"]').each(function() { >+ var id = $(this).data('parent'); >+ if ( id ) { >+ $('#permissions-table').treetable('expandNode', id ); >+ } >+ }); >+ >+ // Behavior that occurs when a checkbox is changed >+ $('input[name="permission"]').on('change', function(){ >+ var id = $(this).closest('tr').attr('id'); >+ >+ if ( id != 'superlibrarian' ) { // Standard behavior >+ if ( $(this).attr('checked') ) { >+ $('#permissions-table').treetable('expandNode', id ); >+ $(`input[data-parent=${id}]`).attr('checked', 'checked' ); >+ } else { >+ $(`input[data-parent=${id}]`).attr('checked', null ); >+ } >+ } else { // Superlibrarian behavior >+ if ( $(this).attr('checked') ) { >+ $(`input[name="permission"][value!="superlibrarian"]`).attr('checked', null ).attr('disabled', 'disabled' ); >+ } else { >+ $(`input[name="permission"][value!="superlibrarian"]`).attr('disabled', null ); >+ } >+ } >+ }); >+ }); >+</script> > </head> > > <body id="pat_member-flags" class="pat"> >@@ -15,74 +52,72 @@ > <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/members/members-home.pl">Patrons</a> › Set permissions for [% patron.surname %], [% patron.firstname %]</div> > > <div id="doc3" class="yui-t2"> >- >- <div id="bd"> >- <div id="yui-main"> >- <div class="yui-b"> >-[% INCLUDE 'members-toolbar.inc' %] >- >-<form method="post" action="/cgi-bin/koha/members/member-flags.pl"> >- <input type="hidden" name="csrf_token" value="[% csrf_token %]" /> >- <input type="hidden" name="member" id="borrowernumber" value="[% patron.borrowernumber %]" /> >- <input type="hidden" name="newflags" value="1" /> >- <h1>Set permissions for [% patron.surname %], [% patron.firstname %]</h1> >- <!-- <ul id="permissionstree"><li class="root">All privileges<ul> --> >- <ul id="permissionstree" class="treeview-grey"> >- <!-- <li class="folder-close">One level down<ul> --> >- [% FOREACH loo IN loop %] >- [% IF ( loo.expand ) %] >- <li class="open"> >- [% ELSE %] >- <li> >- [% END %] >- [% IF ( loo.checked ) %] >- [% IF disable_superlibrarian_privs && loo.bit == 0 %] >- <input type="checkbox" disabled="disabled" class="flag parent superlib" id="flag-[% loo.bit %]_disabled" name="flag" value="[% loo.flag %]" checked="checked" title="The system preference ProtectSuperlibrarianPrivileges is enabled" /> >- <input type="hidden" id="flag-[% loo.bit %]" name="flag" value="[% loo.flag %]" > >- [% ELSE %] >- <input type="checkbox" class="flag parent" id="flag-[% loo.bit %]" name="flag" value="[% loo.flag %]" checked="checked" /> >- [% END %] >- [% ELSE %] >- [% IF disable_superlibrarian_privs && loo.bit == 0 %] >- <input type="checkbox" disabled="disabled" class="flag parent" id="flag-[% loo.bit %]_disabled" name="flag" value="[% loo.flag %]" title="The system preference ProtectSuperlibrarianPrivileges is enabled" /> >- [% ELSE %] >- <input type="checkbox" class="flag parent" id="flag-[% loo.bit %]" name="flag" value="[% loo.flag %]" /> >- [% END %] >- [% END %] >- <label class="permissioncode" for="flag-[% loo.bit %]">[% loo.flag %]</label> >- <span class="permissiondesc">[% PROCESS main_permissions name=loo.flag %]</span> >- [% IF ( loo.sub_perm_loop ) %] >- <ul id="flag-[% loo.bit %]-children"> >- [% FOREACH sub_perm_loo IN loo.sub_perm_loop %] >- <li> >- [% IF ( sub_perm_loo.checked ) %] >- <input type="checkbox" class="flag child" id="[% sub_perm_loo.id %]" name="flag" value="[% sub_perm_loo.perm %]" checked="checked" /> >- [% ELSE %] >- <input type="checkbox" class="flag child" id="[% sub_perm_loo.id %]" name="flag" value="[% sub_perm_loo.perm %]" /> >- [% END %] >- <label class="permissioncode" for="[% sub_perm_loo.id %]">[% sub_perm_loo.code %]</label> >- <span class="permissiondesc">[% PROCESS sub_permissions name=sub_perm_loo.code %]</span> >- </li> >+ <div id="bd"> >+ <div id="yui-main"> >+ <div class="yui-b"> >+ [% INCLUDE 'members-toolbar.inc' %] >+ >+ <form method="post" action="/cgi-bin/koha/members/member-flags.pl"> >+ <input type="hidden" name="csrf_token" value="[% csrf_token %]" /> >+ <input type="hidden" name="borrowernumber" id="borrowernumber" value="[% borrowernumber %]" /> >+ <input type="hidden" name="set_permissions" value="1" /> >+ <h1>Set permissions for [% surname %], [% firstname %]</h1> >+ >+ <table id="permissions-table"> >+ <tr data-tt-id="superlibrarian" id="superlibrarian"> >+ <td> >+ [% IF patron.has_permission( p.code ) %] >+ <input type="checkbox" name="permission" value="superlibrarian" checked="checked"/> >+ [% ELSE %] >+ <input type="checkbox" name="permission" value="superlibrarian"/> >+ [% END %] >+ </td> >+ <td>superlibrarian</td> >+ <td><span class="permissiondesc">[% PROCESS permissions name='superlibrarian' %]</span> >+ </tr> >+ [% FOREACH p IN permissions %] >+ [% NEXT IF p.code == 'superlibrarian' %] >+ <tr data-tt-id="[% p.code %]" id="[% p.code %]"> >+ <td> >+ [% IF patron.has_permission( p.code ) %] >+ <input type="checkbox" name="permission" value="[% p.code %]" checked="checked"/> >+ [% ELSE %] >+ <input type="checkbox" name="permission" value="[% p.code %]"/> >+ [% END %] >+ </td> >+ <td>[% p.code %]</td> >+ <td><span class="permissiondesc">[% PROCESS permissions name=p.code %]</span> >+ </tr> >+ >+ [% FOREACH s IN p.subpermissions() %] >+ <tr data-tt-parent-id="[% p.code %]" data-tt-id="[% s.code %]" id="[% s.code %]"> >+ <td> >+ [% IF patron.has_permission( s.code ) %] >+ <input type="checkbox" name="permission" value="[% s.code %]" checked="checked" data-parent="[% p.code %]"/> >+ [% ELSE %] >+ <input type="checkbox" name="permission" value="[% s.code %]" data-parent="[% p.code %]"/> >+ [% END %] >+ </td> >+ <td>[% s.code %]</td> >+ <td><span class="permissiondesc">[% PROCESS permissions name=s.code %]</span> >+ </tr> >+ [% END %] > [% END %] >- </ul> >- </li> >- [% ELSE %] >- </li> >- [% END %] >- [% END %] >- <!-- </ul></li> --> >- <!-- </ul></li></ul> --> >- </ul> >- >-<fieldset class="action"><input type="submit" value="Save" /> <a class="cancel" href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber %]">Cancel</a></fieldset> >- >-</form> >-</div> >-</div> >+ </table> > >-<div class="yui-b"> >-[% INCLUDE 'circ-menu.inc' %] >-</div> >+ <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" /> >+ >+ <fieldset class="action"> >+ <input type="submit" value="Save" /> >+ <a class="cancel" href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">Cancel</a> >+ </fieldset> >+ </form> >+ </div> >+ </div> >+ >+ <div class="yui-b"> >+ [% INCLUDE 'circ-menu.inc' %] >+ </div> > </div> > > [% MACRO jsinclude BLOCK %] >diff --git a/members/member-flags.pl b/members/member-flags.pl >index c527ff09f4..2786dcf0db 100755 >--- a/members/member-flags.pl >+++ b/members/member-flags.pl >@@ -1,208 +1,138 @@ > #!/usr/bin/perl > >-# script to edit a member's flags >-# Written by Steve Tonnesen >-# July 26, 2002 (my birthday!) >+# Copyright 2017 ByWater Solutions; >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <http://www.gnu.org/licenses>. > > use Modern::Perl; > > use CGI qw ( -utf8 ); > use C4::Output; >-use C4::Auth qw(:DEFAULT :EditPermissions); >+use C4::Auth; > use C4::Context; > use C4::Members; > use C4::Members::Attributes qw(GetBorrowerAttributes); >-#use C4::Acquisitions; > >-use Koha::Patron::Categories; > use Koha::Patrons; >+use Koha::Patron::Images; >+use Koha::Patron::Categories; > >-use C4::Output; > use Koha::Token; >+use Koha::Permissions; > > my $input = new CGI; > > my $flagsrequired = { permissions => 1 }; >-my $member=$input->param('member'); >-my $patron = Koha::Patrons->find( $member ); >-unless ( $patron ) { >- print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$member"); >- exit; >-} >+my $borrowernumber = $input->param('borrowernumber'); >+ >+my $patron = Koha::Patrons->find($borrowernumber); > > my $category_type = $patron->category->category_type; >-my $bor = $patron->unblessed; >-if( $category_type eq 'S' ) { # FIXME Is this really needed? >+my $bor = $patron->unblessed; >+if ( $category_type eq 'S' ) { > $flagsrequired->{'staffaccess'} = 1; > } >-my ($template, $loggedinuser, $cookie) = get_template_and_user({ >+my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >+ { > template_name => "members/member-flags.tt", > query => $input, > type => "intranet", > authnotrequired => 0, > flagsrequired => $flagsrequired, > debug => 1, >-}); >+ } >+); > > my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; > output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); > >-my %member2; >-$member2{'borrowernumber'}=$member; >- >-if ($input->param('newflags')) { >+if ( $input->param('set_permissions') ) { # update permissions > > die "Wrong CSRF token" >- unless Koha::Token->new->check_csrf({ >+ unless Koha::Token->new->check_csrf( >+ { > session_id => scalar $input->cookie('CGISESSID'), >- token => scalar $input->param('csrf_token'), >- }); >+ token => scalar $input->param('csrf_token'), >+ } >+ ); > >+ my @permissions = $input->multi_param('permission'); > >- my $dbh=C4::Context->dbh(); >+ $patron->set_permissions( { permissions => \@permissions } ); > >- my @perms = $input->multi_param('flag'); >- my %all_module_perms = (); >- my %sub_perms = (); >- foreach my $perm (@perms) { >- if ($perm !~ /:/) { >- $all_module_perms{$perm} = 1; >- } else { >- my ($module, $sub_perm) = split /:/, $perm, 2; >- push @{ $sub_perms{$module} }, $sub_perm; >- } >- } >+ print $input->redirect( >+ "/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber"); >+} >+else { # fetch for display >+ my $permissions = Koha::Permissions->search( { parent => undef } ); >+ $template->param( >+ patron => $patron, >+ permissions => $permissions, >+ ); > >- # construct flags >- my $module_flags = 0; >- my $sth=$dbh->prepare("SELECT bit,flag FROM userflags ORDER BY bit"); >- $sth->execute(); >- while (my ($bit, $flag) = $sth->fetchrow_array) { >- if (exists $all_module_perms{$flag}) { >- $module_flags += 2**$bit; >- } >+ # Patron details boilerplate for patron related templates >+ if ( $category_type eq 'C' ) { >+ my $patron_categories = >+ Koha::Patron::Categories->search_limited( { category_type => 'A' }, >+ { order_by => ['categorycode'] } ); >+ $template->param( 'CATCODE_MULTI' => 1 ) >+ if $patron_categories->count > 1; >+ $template->param( 'catcode' => $patron_categories->next ) >+ if $patron_categories->count == 1; > } >- >- $sth = $dbh->prepare("UPDATE borrowers SET flags=? WHERE borrowernumber=?"); >- my $old_flags = $patron->flags // 0; >- if( ( $old_flags == 1 || $module_flags == 1 ) && >- $old_flags != $module_flags ) { >- die "Non-superlibrarian is changing superlibrarian privileges" if !C4::Context->IsSuperLibrarian && C4::Context->preference('ProtectSuperlibrarianPrivileges'); # Interface should not allow this, so we can just die here >- } >- $sth->execute($module_flags, $member); >- >- # deal with subpermissions >- $sth = $dbh->prepare("DELETE FROM user_permissions WHERE borrowernumber = ?"); >- $sth->execute($member); >- $sth = $dbh->prepare("INSERT INTO user_permissions (borrowernumber, module_bit, code) >- SELECT ?, bit, ? >- FROM userflags >- WHERE flag = ?"); >- foreach my $module (keys %sub_perms) { >- next if exists $all_module_perms{$module}; >- foreach my $sub_perm (@{ $sub_perms{$module} }) { >- $sth->execute($member, $sub_perm, $module); >- } >- } >- >- print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=$member"); >-} else { >- >- my $accessflags; >- my $dbh = C4::Context->dbh(); >- # FIXME This needs to be improved to avoid doing the same query >- my $sth = $dbh->prepare("select bit,flag from userflags"); >- $sth->execute; >- while ( my ( $bit, $flag ) = $sth->fetchrow ) { >- if ( $bor->{flags} && $bor->{flags} & 2**$bit ) { >- $accessflags->{$flag} = 1; >- } >- } >- >- my $all_perms = get_all_subpermissions(); >- my $user_perms = get_user_subpermissions($bor->{'userid'}); >- $sth = $dbh->prepare("SELECT bit, flag FROM userflags ORDER BY bit"); >- $sth->execute; >- my @loop; > >- while (my ($bit, $flag) = $sth->fetchrow) { >- my $checked=''; >- if ($accessflags->{$flag}) { >- $checked= 1; >- } >+ $template->param( adultborrower => 1 ) if ( $category_type =~ /^(A|I)$/ ); >+ $template->param( picture => 1 ) if $patron->image; > >- my %row = ( bit => $bit, >- flag => $flag, >- checked => $checked, >+ if ( C4::Context->preference('ExtendedPatronAttributes') ) { >+ my $attributes = GetBorrowerAttributes( $bor->{'borrowernumber'} ); >+ $template->param( >+ ExtendedPatronAttributes => 1, >+ extendedattributes => $attributes > ); >- >- my @sub_perm_loop = (); >- my $expand_parent = 0; >- if ($checked) { >- if (exists $all_perms->{$flag}) { >- $expand_parent = 1; >- foreach my $sub_perm (sort keys %{ $all_perms->{$flag} }) { >- push @sub_perm_loop, { >- id => "${flag}_$sub_perm", >- perm => "$flag:$sub_perm", >- code => $sub_perm, >- checked => 1 >- }; >- } >- } >- } else { >- if (exists $user_perms->{$flag}) { >- $expand_parent = 1; >- # put selected ones first >- foreach my $sub_perm (sort keys %{ $user_perms->{$flag} }) { >- push @sub_perm_loop, { >- id => "${flag}_$sub_perm", >- perm => "$flag:$sub_perm", >- code => $sub_perm, >- checked => 1 >- }; >- } >- } >- # then ones not selected >- if (exists $all_perms->{$flag}) { >- foreach my $sub_perm (sort keys %{ $all_perms->{$flag} }) { >- push @sub_perm_loop, { >- id => "${flag}_$sub_perm", >- perm => "$flag:$sub_perm", >- code => $sub_perm, >- checked => 0 >- } unless exists $user_perms->{$flag} and exists $user_perms->{$flag}->{$sub_perm}; >- } >- } >- } >- $row{expand} = $expand_parent; >- if ($#sub_perm_loop > -1) { >- $row{sub_perm_loop} = \@sub_perm_loop; >- } >- push @loop, \%row; >- } >- >- if ( $patron->is_child ) { >- my $patron_categories = Koha::Patron::Categories->search_limited({ category_type => 'A' }, {order_by => ['categorycode']}); >- $template->param( 'CATCODE_MULTI' => 1) if $patron_categories->count > 1; >- $template->param( 'catcode' => $patron_categories->next->categorycode ) if $patron_categories->count == 1; > } > >-if (C4::Context->preference('ExtendedPatronAttributes')) { >- my $attributes = GetBorrowerAttributes($bor->{'borrowernumber'}); > $template->param( >- ExtendedPatronAttributes => 1, >- extendedattributes => $attributes >+ borrowernumber => $bor->{'borrowernumber'}, >+ cardnumber => $bor->{'cardnumber'}, >+ surname => $bor->{'surname'}, >+ firstname => $bor->{'firstname'}, >+ othernames => $bor->{'othernames'}, >+ categorycode => $bor->{'categorycode'}, >+ category_type => $category_type, >+ categoryname => $bor->{'description'}, >+ address => $bor->{address}, >+ address2 => $bor->{'address2'}, >+ streettype => $bor->{streettype}, >+ city => $bor->{'city'}, >+ state => $bor->{'state'}, >+ zipcode => $bor->{'zipcode'}, >+ country => $bor->{'country'}, >+ phone => $bor->{'phone'}, >+ phonepro => $bor->{'phonepro'}, >+ mobile => $bor->{'mobile'}, >+ email => $bor->{'email'}, >+ emailpro => $bor->{'emailpro'}, >+ branchcode => $bor->{'branchcode'}, >+ is_child => ( $category_type eq 'C' ), >+ RoutingSerials => C4::Context->preference('RoutingSerials'), >+ csrf_token => Koha::Token->new->generate_csrf( >+ { session_id => scalar $input->cookie('CGISESSID'), } >+ ), > ); >-} >- >-$template->param( >- patron => $patron, >- loop => \@loop, >- csrf_token => >- Koha::Token->new->generate_csrf( { session_id => scalar $input->cookie('CGISESSID'), } ), >- disable_superlibrarian_privs => C4::Context->preference('ProtectSuperlibrarianPrivileges') ? !C4::Context->IsSuperLibrarian : 0, >-); > > output_html_with_http_headers $input, $cookie, $template->output; > >-- >2.15.1 (Apple Git-101)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=75543">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20813
:
75542
|
75543
|
75544
|
75545
|
124834
|
124835
|
124836
|
126280
|
126281
|
126282
|
126283
|
126284
|
126285
|
126287
|
126288
|
126289