From 87b3399812ddbd1a33808215e9a85c151bdda481 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Wed, 26 Apr 2023 15:21:39 +0000 Subject: [PATCH] Bug 33363: Add separate permissions for creating and deleting suggestions There are a number of libraries that would like for their staff to be able to submit (and view existing) purchase suggestions from the borrower record, but not give the staff the ability to edit/manage/delete purchase suggestions. Test Plan: 1) Apply this patch 2) Run restart all the things! 3) Run updatedatabase 4) Verify anyone with the suggestions manage permissions now has the create and delete permissions as well 5) Verify anyone without the suggestions manage permission has not recieved the new permissions 6) Enable only the create permission for a librarian 7) Verify that librarian can only create new suggestions ( not manage or delete ) 8) Enable only the manage permission for a librarian 9) Verify that librarian can only manage existing suggestions ( not create or delete ) 10) Enable only the delete permission for a librarian 11) Verify that librarian can only delete suggestions ( not create or manage ) Signed-off-by: Michaela Sieber Signed-off-by: Martin Renvoize Sponsored-by: Cuyahoga County Public Library Signed-off-by: Katrin Fischer --- api/v1/swagger/paths/suggestions.yaml | 19 ++-- .../data/mysql/atomicupdate/bug_33363.pl | 25 ++++++ .../data/mysql/mandatory/userpermissions.sql | 2 + .../prog/en/includes/acquisitions-menu.inc | 2 +- .../prog/en/includes/circ-menu.inc | 2 +- .../intranet-tmpl/prog/en/includes/header.inc | 2 +- .../prog/en/includes/patron-search.inc | 2 +- .../prog/en/includes/permissions.inc | 10 +++ .../modules/members/purchase-suggestions.tt | 4 +- .../prog/en/modules/suggestion/suggestion.tt | 85 ++++++++++++------ members/purchase-suggestions.pl | 2 +- suggestion/suggestion.pl | 87 +++++++++++++------ t/Koha/Auth/Permissions.t | 2 + 13 files changed, 181 insertions(+), 63 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_33363.pl diff --git a/api/v1/swagger/paths/suggestions.yaml b/api/v1/swagger/paths/suggestions.yaml index 6e500bcf4c..2058bb82bf 100644 --- a/api/v1/swagger/paths/suggestions.yaml +++ b/api/v1/swagger/paths/suggestions.yaml @@ -41,7 +41,10 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - suggestions: suggestions_manage + suggestions: + - suggestions_manage + - suggestions_delete + - suggestions_create post: x-mojo-to: Suggestions#add operationId: addSuggestions @@ -100,7 +103,7 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - suggestions: suggestions_manage + suggestions: suggestions_create "/suggestions/{suggestion_id}": get: x-mojo-to: Suggestions#get @@ -139,7 +142,10 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - suggestions: suggestions_manage + suggestions: + - suggestions_manage + - suggestions_delete + - suggestions_create put: x-mojo-to: Suggestions#update operationId: updateSuggestion @@ -229,7 +235,7 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - suggestions: suggestions_manage + suggestions: suggestions_delete /suggestions/managers: get: x-mojo-to: Suggestions#list_managers @@ -281,4 +287,7 @@ $ref: "../swagger.yaml#/definitions/error" x-koha-authorization: permissions: - suggestions: suggestions_manage + suggestions: + - suggestions_manage + - suggestions_delete + - suggestions_create diff --git a/installer/data/mysql/atomicupdate/bug_33363.pl b/installer/data/mysql/atomicupdate/bug_33363.pl new file mode 100755 index 0000000000..6f06c336b4 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_33363.pl @@ -0,0 +1,25 @@ +use Modern::Perl; + +return { + bug_number => "33363", + description => + "Split suggestions_manage into three separate permissions for creating, updating, and deleting suggetions", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do( + q{INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (12, 'suggestions_create', 'Create purchase suggestions')} + ); + $dbh->do( + q{INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (12, 'suggestions_delete', 'Update purchase suggestions')} + ); + + $dbh->do( + q{INSERT IGNORE INTO user_permissions (borrowernumber, module_bit, code) SELECT borrowernumber, 12, 'suggestions_create' FROM borrowers WHERE flags & (1 << 2)} + ); + $dbh->do( + q{INSERT IGNORE INTO user_permissions (borrowernumber, module_bit, code) SELECT borrowernumber, 12, 'suggestions_delete' FROM borrowers WHERE flags & (1 << 2)} + ); + }, +}; diff --git a/installer/data/mysql/mandatory/userpermissions.sql b/installer/data/mysql/mandatory/userpermissions.sql index 4b5c8ebe2c..6252cbd20d 100644 --- a/installer/data/mysql/mandatory/userpermissions.sql +++ b/installer/data/mysql/mandatory/userpermissions.sql @@ -91,7 +91,9 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'delete_invoices', 'Delete invoices'), (11, 'merge_invoices', 'Merge invoices'), (11, 'delete_baskets', 'Delete baskets'), + (12, 'suggestions_create', 'Create purchase suggestions'), (12, 'suggestions_manage', 'Manage purchase suggestions'), + (12, 'suggestions_delete', 'Delete purchase suggestions'), (13, 'edit_additional_contents', 'Write additional contents for the OPAC and staff interfaces (news and HTML customizations)'), (13, 'label_creator', 'Create printable labels and barcodes from catalog and patron data'), (13, 'edit_calendar', 'Define days when the library is closed'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc index 2130e18792..df067e6edf 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/acquisitions-menu.inc @@ -6,7 +6,7 @@
  • Acquisitions home
  • Advanced search
    • [% IF ( CAN_user_acquisition_order_receive ) %]
  • Late orders
    • [% END %] - [% IF ( CAN_user_suggestions_suggestions_manage ) %]
  • Suggestions
    • [% END %] + [% IF ( suggestion && ( CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete ) ) %]
  • Suggestions
    • [% END %]
  • Invoices
    • [% IF Koha.Preference('EDIFACT') && CAN_user_acquisition_edi_manage %]
  • EDIFACT messages
    • diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc index 8c2592c037..9c6d83ea4e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc @@ -226,7 +226,7 @@ [% END %] [% END %] - [% IF CAN_user_suggestions_suggestions_manage %] + [% IF CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete %] [% IF ( suggestionsview ) %]
  • [% ELSE %]
  • [% END %] Purchase suggestions
    • diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc index 445e2893ca..cad6fa53e3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc @@ -59,7 +59,7 @@ [% IF ( CAN_user_reports ) %]
  • Reports
    • [% END %] - [% IF ( CAN_user_suggestions_suggestions_manage ) %] + [% IF ( CAN_user_suggestions_suggestions_create || CAN_user_suggestions_suggestions_manage || CAN_user_suggestions_suggestions_delete ) %]
  • Suggestions
    • [% END %] [% IF ( CAN_user_tools || CAN_user_clubs ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc index c8dd64c124..a0d53600e3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc @@ -127,7 +127,7 @@ [% END %] [% IF filter == 'suggestions_managers' %] -
    Only staff with superlibrarian or suggestions_manage permissions are returned in the search results
    +
    Only staff with superlibrarian or full suggestions permissions are returned in the search results
    [% ELSIF filter == 'orders_managers' OR filter == 'baskets_managers' %]
    Only staff with superlibrarian or acquisitions permissions (or order_manage permission if granular permissions are enabled) are returned in the search results
    [% ELSIF filter == 'funds_owners' OR filter == 'funds_users' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc index b8f2c2b040..b8d645af83 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc @@ -437,11 +437,21 @@ Add manual credits to a patron account ([% name | html %]) + [%- CASE 'suggestions_create' -%] + + Create purchase suggestions + + ([% name | html %]) [%- CASE 'suggestions_manage' -%] Manage purchase suggestions ([% name | html %]) + [%- CASE 'suggestions_delete' -%] + + Delete purchase suggestions + + ([% name | html %]) [%- CASE 'budget_add_del' -%] Add and delete funds (but can't modify funds) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt index 0737529d79..f814d3bbd6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt @@ -44,7 +44,9 @@

    Purchase suggestions

    - New purchase suggestion + [% IF CAN_user_suggestions_suggestions_create %] + New purchase suggestion + [% END %]
    [% IF suggestions.size %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt index 0b0940e6ff..4b5b7eb03f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt @@ -97,13 +97,17 @@ [% INCLUDE 'messages.inc' %]
    - Edit -
    - [% INCLUDE 'csrf-token.inc' %] - - - -
    + [% IF CAN_user_suggestions_suggestions_manage %] + Edit + [% END %] + [% IF CAN_user_suggestions_suggestions_delete %] +
    + [% INCLUDE 'csrf-token.inc' %] + + + +
    + [% END %]
    @@ -338,6 +342,12 @@ A similar document already exists: [% m.title | html %]. Click on "Confirm your suggestion" to ignore this message. [% CASE 'manager_not_enough_permissions' %] The manager you selected does not have sufficient permissions. + [% CASE 'no_manage_permission' %] + You do not have permissions to manage suggestions + [% CASE 'no_create_permission' %] + You do not have permissions to create suggestions + [% CASE 'no_delete_permission' %] + You do not have permissions to delete suggestions [% CASE %] [% m.code | html %] [% END %] @@ -558,7 +568,11 @@
    - You + [% IF CAN_user_suggestions_suggestions_manage %] + You + [% ELSE %] + Nobody + [% END %] [% IF managedby_patron.borrowernumber && logged_in_user.borrowernumber != managedby_patron.borrowernumber %] | Previously was [% INCLUDE 'patron-title.inc' patron=managedby_patron hide_patron_infos_if_needed=1 %] [% Branches.GetName( managedby_patron.branchcode ) | html %] ([% managedby_patron.category.description | html %]) @@ -568,7 +582,9 @@ [% IF managedby_patron.borrowernumber && logged_in_user.borrowernumber != managedby_patron.borrowernumber %] Keep existing manager [% END %] - + [% IF CAN_user_suggestions_suggestions_manage %] + + [% END %]
    @@ -649,7 +665,9 @@ [% IF op == 'else' %]
    - New purchase suggestion + [% IF CAN_user_suggestions_suggestions_create %] + New purchase suggestion + [% END %]

    Suggestions management

    @@ -843,23 +861,29 @@ [% END %] -
    - Edit - -
    + [% IF CAN_user_suggestions_suggestions_manage %] +
    + Edit + +
    + [% ELSIF CAN_user_suggestions_suggestions_delete %] + + [% END %] [% END # /FOREACH s %] @@ -872,6 +896,7 @@
    + [% IF CAN_user_suggestions_suggestions_manage %]
    @@ -964,7 +989,9 @@
    + [% END %] + [% IF CAN_user_suggestions_suggestions_delete %]
    Delete selected @@ -975,6 +1002,9 @@
    + [% END %] + + [% IF CAN_user_suggestions_suggestions_manage %]
    Archive selected @@ -985,6 +1015,7 @@
    + [% END %] [% ELSE %] diff --git a/members/purchase-suggestions.pl b/members/purchase-suggestions.pl index 0268582bef..c56a493d9c 100755 --- a/members/purchase-suggestions.pl +++ b/members/purchase-suggestions.pl @@ -32,7 +32,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { template_name => "members/purchase-suggestions.tt", query => $input, type => "intranet", - flagsrequired => { suggestions => 'suggestions_manage' }, + flagsrequired => { suggestions => '*' }, } ); diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl index 3c68cf1384..62a82a8058 100755 --- a/suggestion/suggestion.pl +++ b/suggestion/suggestion.pl @@ -125,13 +125,15 @@ unless ( $op eq 'cud-save' ) { } my ( $template, $borrowernumber, $cookie, $userflags ) = get_template_and_user( - { - template_name => "suggestion/suggestion.tt", - query => $input, - type => "intranet", - flagsrequired => { suggestions => 'suggestions_manage' }, - } - ); + { + template_name => "suggestion/suggestion.tt", + query => $input, + type => "intranet", + flagsrequired => { suggestions => '*' }, + } +); + +my $librarian = Koha::Patrons->find($borrowernumber); $borrowernumber = $input->param('borrowernumber') if ( $input->param('borrowernumber') ); $template->param('borrowernumber' => $borrowernumber); @@ -141,8 +143,12 @@ my $branchfilter = $input->param('branchcode') || C4::Context->userenv->{'branch ## Operations ## +my @messages; if ( $op =~ /cud-save/ ) { + output_and_exit_if_error($input, $cookie, $template, { check => 'csrf_token' }); + my @messages; + my $biblio = MarcRecordFromNewSuggestion({ title => $suggestion_only->{title}, author => $suggestion_only->{author}, @@ -201,7 +207,12 @@ if ( $op =~ /cud-save/ ) { if exists $suggestion_only->{branchcode} && $suggestion_only->{branchcode} eq ""; - &ModSuggestion($suggestion_only); + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) { + &ModSuggestion($suggestion_only); + } else { + push @messages, { type => 'error', code => 'no_manage_permission' }; + $template->param( messages => \@messages, ); + } if ( $notify ) { my $patron = Koha::Patrons->find( $suggestion_only->{managedby} ); @@ -241,7 +252,12 @@ if ( $op =~ /cud-save/ ) { } else { ## Adding some informations related to suggestion - Koha::Suggestion->new($suggestion_only)->store(); + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_create' } ) ) { + Koha::Suggestion->new($suggestion_only)->store(); + } else { + push @messages, { type => 'error', code => 'no_delete_permission' }; + $template->param( messages => \@messages ); + } } # empty fields, to avoid filter in "SearchSuggestion" } @@ -303,17 +319,28 @@ elsif ($op eq "cud-update_status" ) { $suggestion->{reason} = $reason; } - foreach my $suggestionid (@editsuggestions) { - next unless $suggestionid; - $suggestion->{suggestionid} = $suggestionid; - &ModSuggestion($suggestion); + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) { + foreach my $suggestionid (@editsuggestions) { + next unless $suggestionid; + $suggestion->{suggestionid} = $suggestionid; + &ModSuggestion($suggestion); + } + redirect_with_params($input); + } else { + push @messages, { type => 'error', code => 'no_manage_permission' }; + $template->param( messages => \@messages, ); } redirect_with_params($input); -}elsif ($op eq "cud-delete" ) { - foreach my $delete_field (@editsuggestions) { - &DelSuggestion( $borrowernumber, $delete_field,'intranet' ); +} elsif ($op eq "cud-delete" ) { + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_delete' } ) ) { + foreach my $delete_field (@editsuggestions) { + &DelSuggestion( $borrowernumber, $delete_field, 'intranet' ); + } + redirect_with_params($input); + } else { + push @messages, { type => 'error', code => 'no_delete_permission' }; + $template->param( messages => \@messages, ); } - redirect_with_params($input); } elsif ($op eq "cud-archive" ) { Koha::Suggestions->find($_)->update({ archived => 1 }) for @editsuggestions; @@ -324,18 +351,28 @@ elsif ($op eq "cud-unarchive" ) { redirect_with_params($input); } elsif ( $op eq 'cud-update_itemtype' ) { - foreach my $suggestionid (@editsuggestions) { - next unless $suggestionid; - &ModSuggestion({ suggestionid => $suggestionid, itemtype => $new_itemtype }); + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) { + foreach my $suggestionid (@editsuggestions) { + next unless $suggestionid; + &ModSuggestion({ suggestionid => $suggestionid, itemtype => $new_itemtype }); + } + redirect_with_params($input); + } else { + push @messages, { type => 'error', code => 'no_manage_permission' }; + $template->param( messages => \@messages, ); } - redirect_with_params($input); } elsif ( $op eq 'cud-update_manager' ) { - foreach my $suggestionid (@editsuggestions) { - next unless $suggestionid; - &ModSuggestion({ suggestionid => $suggestionid, managedby => $sugg_managedby }); + if ( $librarian->has_permission( { 'suggestions' => 'suggestions_manage' } ) ) { + foreach my $suggestionid (@editsuggestions) { + next unless $suggestionid; + &ModSuggestion({ suggestionid => $suggestionid, managedby => $sugg_managedby }); + } + redirect_with_params($input); + } else { + push @messages, { type => 'error', code => 'no_manage_permission' }; + $template->param( messages => \@messages, ); } - redirect_with_params($input); } elsif ( $op eq 'show' ) { $suggestion_ref=&GetSuggestion($$suggestion_ref{'suggestionid'}); diff --git a/t/Koha/Auth/Permissions.t b/t/Koha/Auth/Permissions.t index 9662f5ba3b..76e3f26946 100755 --- a/t/Koha/Auth/Permissions.t +++ b/t/Koha/Auth/Permissions.t @@ -244,7 +244,9 @@ subtest 'superlibrarian tests' => sub { 'CAN_user_stockrotation_manage_rota_items' => 1, 'CAN_user_stockrotation_manage_rotas' => 1, 'CAN_user_stockrotation' => 1, + 'CAN_user_suggestions_suggestions_create' => 1, 'CAN_user_suggestions_suggestions_manage' => 1, + 'CAN_user_suggestions_suggestions_delete' => 1, 'CAN_user_suggestions' => 1, 'CAN_user_superlibrarian' => 1, 'CAN_user_tools_access_files' => 1, -- 2.39.5