Bugzilla – Attachment 126282 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), 45.78 KB, created by
Kyle M Hall (khall)
on 2021-10-14 14:05:15 UTC
(
hide
)
Description:
Bug 20813: Revamp user permissions system
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2021-10-14 14:05:15 UTC
Size:
45.78 KB
patch
obsolete
>From 055e60d35179accd5ad8e40d1ebd5fbc6f1ab911 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 | 141 +++++----- > 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 +- > .../prog/en/includes/permissions.inc | 7 +- > .../prog/en/modules/members/member-flags.tt | 174 +++++++------ > members/member-flags.pl | 241 +++++++----------- > 13 files changed, 549 insertions(+), 330 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 dc733b221a8..8d6c77d85ab 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -853,7 +853,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 816ae48ac16..f9b72ae5ad6 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -24,27 +24,29 @@ use Carp qw( croak ); > use Digest::MD5 qw( md5_base64 ); > use CGI::Session; > >+use Encode; >+use List::MoreUtils qw( any ); >+use Net::CIDR; >+ >+use C4::Auth_with_shibboleth qw( shib_ok get_login_shib login_shib_url logout_shib checkpw_shib ); > use C4::Context; >-use C4::Templates; # to get the template > use C4::Languages; >+use C4::Log qw( logaction ); > use C4::Search::History; >-use Koha; >-use Koha::Logger; >-use Koha::Caches; >+use C4::Templates; # to get the template > use Koha::AuthUtils qw( get_script_name hash_password ); >+use Koha::Caches; >+use Koha::Cash::Registers; > use Koha::Checkouts; > use Koha::DateUtils qw( dt_from_string ); >-use Koha::Library::Groups; >-use Koha::Libraries; >-use Koha::Cash::Registers; > use Koha::Desks; >-use Koha::Patrons; >+use Koha::Libraries; >+use Koha::Library::Groups; >+use Koha::Logger; > use Koha::Patron::Consents; >-use List::MoreUtils qw( any ); >-use Encode; >-use C4::Auth_with_shibboleth qw( shib_ok get_login_shib login_shib_url logout_shib checkpw_shib ); >-use Net::CIDR; >-use C4::Log qw( logaction ); >+use Koha::Patrons; >+use Koha::Permissions; >+use Koha; > > # use utf8; > >@@ -1125,7 +1127,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, email > FROM borrowers > LEFT JOIN branches on borrowers.branchcode=branches.branchcode >@@ -1141,7 +1143,7 @@ sub checkauth { > } > } > if ( $sth->rows ) { >- ( $borrowernumber, $firstname, $surname, $userflags, >+ ( $borrowernumber, $firstname, $surname, > $branchcode, $branchname, $emailaddress ) = $sth->fetchrow; > } > >@@ -1209,7 +1211,6 @@ sub checkauth { > $session->param( 'branchname', $branchname ); > $session->param( 'desk_id', $desk_id); > $session->param( 'desk_name', $desk_name); >- $session->param( 'flags', $userflags ); > $session->param( 'emailaddress', $emailaddress ); > $session->param( 'ip', $session->remote_addr() ); > $session->param( 'lasttime', time() ); >@@ -1955,35 +1956,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; > } > } >@@ -2007,47 +2008,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; > } > >@@ -2079,20 +2072,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 >@@ -2100,25 +2080,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 >@@ -2180,13 +2157,13 @@ sub _dispatch { > sub haspermission { > my ( $userid, $flagsrequired ) = @_; > >- #Koha::Exceptions::WrongParameter->throw('$flagsrequired should not be undef') >- # unless defined($flagsrequired); >+ my $flags = getuserflags( { userid => $userid } ); > >- my $sth = C4::Context->dbh->prepare("SELECT flags FROM borrowers WHERE userid=?"); >- $sth->execute($userid); >- my $row = $sth->fetchrow(); >- my $flags = getuserflags( $row, $userid ); >+ if ( $userid eq C4::Context->config('user') ) { >+ >+ # Super User Account from /etc/koha.conf >+ $flags->{'superlibrarian'} = 1; >+ } > > return $flags unless defined($flagsrequired); > return $flags if $flags->{superlibrarian}; >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index 9624650be14..ae23f9f8b27 100644 >--- a/C4/Budgets.pm >+++ b/C4/Budgets.pm >@@ -1037,8 +1037,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 9fd93e8c441..731acdc03c4 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -43,6 +43,7 @@ use Koha::Patron::Categories; > use Koha::Patron::Debarments; > use Koha::Patron::HouseboundProfile; > use Koha::Patron::HouseboundRole; >+use Koha::Patron::Permissions; > use Koha::Patron::Images; > use Koha::Patron::Modifications; > use Koha::Patron::Relationships; >@@ -1910,6 +1911,55 @@ sub queue_notice { > > =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 00000000000..04ef51e9ff9 >--- /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 00000000000..e3aaa0538de >--- /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 00000000000..798702cea1d >--- /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 00000000000..2be4cfb3241 >--- /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 55ffc379fbd..beaaccc70e0 100755 >--- a/about.pl >+++ b/about.pl >@@ -46,6 +46,7 @@ use Koha::BiblioFrameworks; > use Koha::Email; > use Koha::Patron::Categories; > use Koha::Patrons; >+use Koha::Patron::Permissions; > use Koha::Caches; > use Koha::Config::SysPrefs; > use Koha::Illrequest::Config; >@@ -417,21 +418,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 6b0d8c12ac5..d2a5385f6fe 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc >@@ -47,7 +47,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 | html %]">Set permissions</a></li> >+ <li><a id="patronflags" href="/cgi-bin/koha/members/member-flags.pl?borrowernumber=[% patron.borrowernumber | html %]">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 cc253be52e1..639c241c3ba 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 class="main_permission superlibrarian_permission">Access to all librarian functions</span> >@@ -75,11 +75,6 @@ > [%- CASE 'problem_reports' -%] > <span class="main_permission problem_reports_permission">OPAC problem reports management</span> > <span class="permissioncode">([% name | html %])</span> >- [%- END -%] >-[%- END -%] >- >-[%- BLOCK sub_permissions -%] >- [% SWITCH name %] > [%- CASE 'circulate_remaining_permissions' -%] > <span class="sub_permission circulate_remaining_permissions_subpermission"> > Remaining circulation permissions >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 6acb98bceb6..a1d9816d60d 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 >@@ -8,6 +8,43 @@ > Set permissions for [% patron.surname | html %], [% patron.firstname | html %] › Patrons › Koha > </title> > [% 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"> >@@ -35,89 +72,74 @@ > <div class="col-sm-10 col-sm-push-2"> > <main> > >-[% INCLUDE 'members-toolbar.inc' %] >+<div id="doc3" class="yui-t2"> >+ <div id="bd"> >+ <div id="yui-main"> >+ <div class="yui-b"> >+ [% INCLUDE 'members-toolbar.inc' %] > >-<form id="flag_form" method="post" action="/cgi-bin/koha/members/member-flags.pl"> >- <input type="hidden" name="csrf_token" value="[% csrf_token | html %]" /> >- <input type="hidden" name="member" id="borrowernumber" value="[% patron.borrowernumber | html %]" /> >- <input type="hidden" name="newflags" value="1" /> >- <h1>Set permissions for [% patron.surname | html %], [% patron.firstname | html %]</h1> >+ <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> > >- <div id="permissionstree"> >- <div id="permissions_toolbar" class="btn-toolbar"> >- <button type="submit" class="btn btn-default"><i class="fa fa-save"></i> Save</button> >- <a class="btn btn-default" href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | html %]"><i class="fa fa-remove"></i> Cancel</a> >- <a class="toggleall toggleall_on btn btn-link" href="#"><i class="fa fa-plus-square-o"></i> Show all</a> >- <a class="toggleall toggleall_off btn btn-link" href="#"><i class="fa fa-minus-square-o"></i> Hide all</a> >- <a id="CheckAllFlags" class="btn btn-link" href="#"> >- <i class="fa fa-check"></i> Select all >- </a> >- <a id="UncheckAllFlags" class="btn btn-link" href="#"> >- <i class="fa fa-remove"></i> Clear all >- </a> >- <div class="btn-group pull-right"> >- Filter: <input type="text" name="permissions_filter" id="permissions_filter" size="20" /> >- <a href="#" id="clear_filter" style="display:none"><i class="fa fa-remove"></i></a> >- </div> >- </div> >- <div class="permissions"> >- [% FOREACH loo IN loop %] >- [% IF ( loo.expand ) %] >- <div id="parent-flag-[% loo.flag | html %]" class="open parent"> >- [% ELSE %] >- <div id="parent-flag-[% loo.flag | html %]" class="parent"> >- [% END %] >- [% IF ( loo.checked ) %] >- [% IF disable_superlibrarian_privs && loo.bit == 0 %] >- <input type="checkbox" disabled="disabled" data-bit="0" id="flag-0" class="parent flag superlib" name="flag" value="[% loo.flag | html %]" checked="checked" title="The system preference ProtectSuperlibrarianPrivileges is enabled" /> >- <input type="hidden" id="flag-0" name="flag" value="[% loo.flag | html %]" > >- [% ELSE %] >- <input type="checkbox" data-bit="[% loo.bit | html %]" id="flag-[% loo.bit | html %]" class="parent flag" name="flag" value="[% loo.flag | html %]" checked="checked" /> >- [% END %] >- [% ELSE %] >- [% IF disable_superlibrarian_privs && loo.bit == 0 %] >- <input type="checkbox" disabled="disabled" class="flag parent" data-bit="0" id="flag-0" name="flag" value="[% loo.flag | html %]" title="The system preference ProtectSuperlibrarianPrivileges is enabled" /> >- [% ELSE %] >- <input type="checkbox" class="flag parent" data-bit="[% loo.bit | html %]" id="flag-[% loo.bit | html %]" name="flag" value="[% loo.flag | html %]" /> >- [% END %] >- [% END # /IF loo.checked %] >- <label class="permissiondesc" for="flag-[% loo.bit | html %]"> >- [% PROCESS main_permissions name=loo.flag %] >- </label> >- [% IF ( loo.flag == "superlibrarian" ) %]<div class="hint superlibrarian-hint">This permission grants access to all areas. If selected, specific sub-permissions cannot be selected.</div>[% END %] >- [% IF ( loo.sub_perm_loop ) %] >- <a class="toggle-[% loo.bit | html %] togglechildren_off" href="#" data-bit="[% loo.bit | html %]"><i class="fa fa-minus-square-o"></i> Hide details</a> >- <a class="toggle-[% loo.bit | html %] togglechildren_on" href="#" data-bit="[% loo.bit | html %]"><i class="fa fa-plus-square-o"></i> Show details</a> >- <div class="children" id="flag-[% loo.bit | html %]-children"> >- [% FOREACH sub_perm_loo IN loo.sub_perm_loop %] >- <div class="child-flags"> >- [% IF ( sub_perm_loo.checked ) %] >- <input type="checkbox" id="[% sub_perm_loo.id | html %]" class="child flag" name="flag" value="[% sub_perm_loo.perm | html %]" checked="checked" /> >+ <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" id="[% sub_perm_loo.id | html %]" class="child flag" name="flag" value="[% sub_perm_loo.perm | html %]" /> >+ <input type="checkbox" name="permission" value="[% p.code %]"/> > [% END %] >- <label class="permissiondesc" for="[% sub_perm_loo.id | html %]"> >- [% PROCESS sub_permissions name=sub_perm_loo.code %] >- </label> >- </div> >+ </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 %] >- </div> <!-- /#flag-[% loo.bit | html %]-children --> >- [% END # /IF loo.sub_perm_loop %] >- </div> <!-- /.parent --> >- [% END # /FOREACH loo %] >- </div> >- </div> <!-- /#permissionstree --> >-</form> >+ [% END %] >+ </table> > >- </main> >- </div> <!-- /.col-sm-10.col-sm-push-2 --> >+ <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="col-sm-2 col-sm-pull-10"> >- <aside> >- [% INCLUDE 'circ-menu.inc' %] >- </aside> >- </div> <!-- /.col-sm-2.col-sm-pull-10 --> >- </div> <!-- /.row --> >+ <div class="yui-b"> >+ [% INCLUDE 'circ-menu.inc' %] >+ </div> >+</div> > > [% MACRO jsinclude BLOCK %] > [% INCLUDE 'str/members-menu.inc' %] >diff --git a/members/member-flags.pl b/members/member-flags.pl >index 900a176ad51..a63df857edf 100755 >--- a/members/member-flags.pl >+++ b/members/member-flags.pl >@@ -1,190 +1,135 @@ > #!/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 qw( output_and_exit_if_error output_and_exit output_html_with_http_headers ); >-use C4::Auth qw( get_template_and_user get_all_subpermissions get_user_subpermissions ); >-use C4::Context; > >+use C4::Auth; >+use C4::Context; >+use C4::Members::Attributes qw(GetBorrowerAttributes); >+use C4::Members; >+use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers ); >+use C4::Output; > use Koha::Patron::Categories; >+use Koha::Patron::Images; > use Koha::Patrons; >- >-use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers ); >+use Koha::Permissions; > use Koha::Token; > > my $input = CGI->new; > > 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", > flagsrequired => $flagsrequired, >-}); >+ } >+); > > my $logged_in_user = Koha::Patrons->find( $loggedinuser ); > 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 > > output_and_exit( $input, $cookie, $template, 'wrong_csrf_token' ) > 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 $dbh=C4::Context->dbh(); >+ my @permissions = $input->multi_param('permission'); > >- 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; >- } >- } >+ $patron->set_permissions( { 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; >- } >- } >- >- $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; >- } >+ 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, >+ ); >+ >+ # 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; > } > >- 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; >- } >- next if ( ( $row{flag} eq 'cash_management' ) && !C4::Context->preference('UseCashRegisters') ); >- push @loop, \%row; > } > >-$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, >-); >+ $template->param( >+ 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'), } >+ ), >+ ); > > output_html_with_http_headers $input, $cookie, $template->output; > >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
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