Bugzilla – Attachment 194088 Details for
Bug 10190
Overdue notice triggers based on item type
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10190: Enforce manage_circ_rules_from_any_libraries permission
b1724b3.patch (text/plain), 18.07 KB, created by
Martin Renvoize (ashimema)
on 2026-02-27 07:16:43 UTC
(
hide
)
Description:
Bug 10190: Enforce manage_circ_rules_from_any_libraries permission
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-27 07:16:43 UTC
Size:
18.07 KB
patch
obsolete
>From b1724b37096ff5efd98f3cbf3a88ba4eed6b6188 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Fri, 27 Feb 2026 07:06:24 +0000 >Subject: [PATCH] Bug 10190: Enforce manage_circ_rules_from_any_libraries > permission > >- Add OR-type x-koha-authorization to /circulation_rules, /circulation_rules/kinds, > and /circulation_rules/config endpoints so users with either > circulate:circulate_remaining_permissions OR parameters:manage_circ_rules > can access them >- Enforce manage_circ_rules_from_any_libraries in the set_rules controller: > users without this permission may only set rules for their own library and > cannot set default (*) rules >- Add explicit use C4::Auth qw(haspermission) import to CirculationRules.pm >- Lock the library selector in the Vue UI for users who lack > manage_circ_rules_from_any_libraries, defaulting them to their own library >- Update set_rules() tests to cover the new permission restrictions >--- > Koha/REST/V1/CirculationRules.pm | 16 ++++- > api/v1/swagger/paths/circulation_rules.yaml | 3 + > .../paths/circulation_rules_config.yaml | 1 + > .../en/modules/admin/circulation_triggers.tt | 1 + > .../CirculationTriggersFormAdd.vue | 25 +++++++- > .../CirculationTriggersList.vue | 35 ++++++++--- > .../Admin/CirculationTriggers/Main.vue | 4 +- > .../prog/js/vue/stores/circulation-rules.js | 22 ++++++- > t/db_dependent/api/v1/circulation_rules.t | 59 ++++++++++++++++++- > 9 files changed, 147 insertions(+), 19 deletions(-) > >diff --git a/Koha/REST/V1/CirculationRules.pm b/Koha/REST/V1/CirculationRules.pm >index 23f77a0681d..a42a6c8a71c 100644 >--- a/Koha/REST/V1/CirculationRules.pm >+++ b/Koha/REST/V1/CirculationRules.pm >@@ -19,6 +19,7 @@ use Modern::Perl; > > use Mojo::Base 'Mojolicious::Controller'; > >+use C4::Auth qw( haspermission ); > use Koha::CirculationRules; > > =head1 API >@@ -240,6 +241,19 @@ sub set_rules { > ) unless $library; > } > >+ my $logged_in_user = $c->stash('koha.user'); >+ unless ( haspermission( $logged_in_user->userid, { parameters => 'manage_circ_rules_from_any_libraries' } ) ) { >+ if ( !defined $branchcode || $branchcode ne $logged_in_user->branchcode ) { >+ return $c->render( >+ status => 403, >+ openapi => { >+ error => >+ 'manage_circ_rules_from_any_libraries permission is required to manage circulation rules for libraries other than your own' >+ } >+ ); >+ } >+ } >+ > if ( $patron_category eq '*' ) { > $patron_category = undef; > } else { >@@ -326,7 +340,7 @@ sub config { > my $c = shift->openapi->valid_input or return; > > my $patron = $c->stash('koha.user'); >- my $userflags = C4::Auth::haspermission( $patron->userid ); >+ my $userflags = haspermission( $patron->userid ); > my $permissions = Koha::Auth::Permissions->get_authz_from_flags( { flags => $userflags } ); > > return $c->render( >diff --git a/api/v1/swagger/paths/circulation_rules.yaml b/api/v1/swagger/paths/circulation_rules.yaml >index b08e1f6bf7f..3767932b181 100644 >--- a/api/v1/swagger/paths/circulation_rules.yaml >+++ b/api/v1/swagger/paths/circulation_rules.yaml >@@ -77,6 +77,7 @@ > x-koha-authorization: > permissions: > - circulate: circulate_remaining_permissions >+ - parameters: manage_circ_rules > put: > x-mojo-to: CirculationRules#set_rules > operationId: setCirculationRules >@@ -131,6 +132,7 @@ > x-koha-authorization: > permissions: > - circulate: circulate_remaining_permissions >+ - parameters: manage_circ_rules > /circulation_rules/kinds: > get: > x-mojo-to: CirculationRules#get_kinds >@@ -169,3 +171,4 @@ > x-koha-authorization: > permissions: > - circulate: circulate_remaining_permissions >+ - parameters: manage_circ_rules >diff --git a/api/v1/swagger/paths/circulation_rules_config.yaml b/api/v1/swagger/paths/circulation_rules_config.yaml >index 6d8c7bf7f90..67c9003b714 100644 >--- a/api/v1/swagger/paths/circulation_rules_config.yaml >+++ b/api/v1/swagger/paths/circulation_rules_config.yaml >@@ -36,3 +36,4 @@ > x-koha-authorization: > permissions: > - circulate: circulate_remaining_permissions >+ - parameters: manage_circ_rules >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt >index 1d30f010470..c42a87abb65 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt >@@ -34,6 +34,7 @@ > const letters = [% To.json(letters) | $raw %]; > const default_view = "[%- IF Koha.Preference('DefaultToLoggedInLibraryOverdueTriggers') -%][% Branches.GetLoggedInBranchcode | html %][%- ELSE -%]*[%- END -%]"; > const from_branch = "[% Koha.Preference('OverdueNoticeFrom') | html %]"; >+ const logged_in_library_id = "[% Branches.GetLoggedInBranchcode | html %]"; > </script> > [% Asset.js("js/vue/dist/admin/circulation_triggers.js") | $raw %] > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue >index 5ceaf3c7788..a0422292ed2 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue >+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue >@@ -36,8 +36,19 @@ > v-model="context.library_id" > label="name" > :reduce="lib => lib.library_id" >- :options="libraries" >- :disabled="editMode !== 'confirmContext'" >+ :options=" >+ canManageAnyLibrary >+ ? libraries >+ : libraries.filter( >+ lib => >+ lib.library_id === >+ logged_in_library_id >+ ) >+ " >+ :disabled=" >+ editMode !== 'confirmContext' || >+ !canManageAnyLibrary >+ " > > > <template #search="{ attributes, events }"> > <input >@@ -442,6 +453,8 @@ export default { > patronCategories, > triggerCounts, > storeInitialized, >+ canManageAnyLibrary, >+ logged_in_library_id, > } = storeToRefs(circRulesStore); > > return { >@@ -458,6 +471,8 @@ export default { > updateCircRuleSets, > hasConflict, > storeInitialized, >+ canManageAnyLibrary, >+ logged_in_library_id, > }; > }, > data() { >@@ -659,7 +674,11 @@ export default { > this.ruleSetInitialized = true; > }, > setContext(query) { >- this.context.library_id = query.library_id ?? "*"; >+ if (!this.canManageAnyLibrary && this.logged_in_library_id) { >+ this.context.library_id = this.logged_in_library_id; >+ } else { >+ this.context.library_id = query.library_id ?? "*"; >+ } > this.context.item_type_id = query.item_type_id ?? "*"; > this.context.patron_category_id = query.patron_category_id ?? "*"; > }, >diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersList.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersList.vue >index 42e2b6c37e0..989f145c895 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersList.vue >+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersList.vue >@@ -138,6 +138,13 @@ > Filter by > <span style="color: blue; font-weight: bold">context</span> > </legend> >+ <p v-if="!canManageAnyLibrary" class="alert alert-info"> >+ {{ >+ $__( >+ "You can only manage circulation rules for your own library." >+ ) >+ }} >+ </p> > <table> > <thead> > <tr> >@@ -154,11 +161,20 @@ > v-model="currentLibraryId" > label="name" > :reduce="lib => lib.library_id" >- :options="libraries" >+ :options=" >+ canManageAnyLibrary >+ ? libraries >+ : libraries.filter( >+ lib => >+ lib.library_id === >+ logged_in_library_id >+ ) >+ " > @update:modelValue=" > filterRuleSetsbySearchParam() > " > :clearable="false" >+ :disabled="!canManageAnyLibrary" > placeholder="Default rules for all libraries" > > > <template #search="{ attributes, events }"> >@@ -353,6 +369,8 @@ export default { > librariesWithRules, > storeInitialized, > metaInitialized, >+ canManageAnyLibrary, >+ logged_in_library_id, > } = storeToRefs(circRulesStore); > > return { >@@ -375,6 +393,8 @@ export default { > storeInitialized, > metaInitialized, > from_branch, >+ canManageAnyLibrary, >+ logged_in_library_id, > }; > }, > data() { >@@ -487,15 +507,12 @@ export default { > ) { > if (!this.metaInitialized) { > await new Promise(resolve => { >- const stop = this.$watch( >- "metaInitialized", >- val => { >- if (val) { >- stop(); >- resolve(); >- } >+ const stop = this.$watch("metaInitialized", val => { >+ if (val) { >+ stop(); >+ resolve(); > } >- ); >+ }); > }); > } > await this.$nextTick(); >diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/Main.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/Main.vue >index 69fa7ac272d..62f7fa15785 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/Main.vue >+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/Main.vue >@@ -29,7 +29,7 @@ import { inject } from "vue"; > export default { > setup() { > const circRulesStore = inject("circRulesStore"); >- circRulesStore.init(default_view); >+ circRulesStore.init(default_view, logged_in_library_id); > letters.unshift({ > name: "No letter", > code: "", >@@ -52,7 +52,7 @@ export default { > > <style> > .page-section.bg-info { >- background-color: #DAECFB !important; >+ background-color: #daecfb !important; > } > > form .v-select { >diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js b/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js >index d5c63b07218..f3214ad25e7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js >@@ -3,7 +3,7 @@ import { $__ } from "../i18n"; > import { APIClient } from "../fetch/api-client.js"; > import { isEqual } from "lodash"; > import { permissionsActions } from "../composables/permissions"; >-import { reactive, toRefs } from "vue"; >+import { reactive, toRefs, computed } from "vue"; > > // NOTES ON RULE SETS TYPES > // exhaustive: includes 'pure fallback' rules sets for contexts that no rules match. >@@ -26,6 +26,7 @@ export const useCircRulesStore = defineStore("circRules", () => { > libraries: [], > patronCategories: [], > userPermissions: null, >+ logged_in_library_id: null, > letters: [], > ruleSuffixes: ["delay", "notice", "mtt", "restrict", "has_rules"], > transportTypes: [ >@@ -43,13 +44,27 @@ export const useCircRulesStore = defineStore("circRules", () => { > storeInitialized: false, > }); > >+ const canManageAnyLibrary = computed( >+ () => >+ !store.userPermissions || >+ !!store.userPermissions >+ .CAN_user_parameters_manage_circ_rules_from_any_libraries >+ ); >+ > const actions = { > // controllers >- async init(defaultLibraryId = "*") { >+ async init(defaultLibraryId = "*", loggedInLibraryId = null) { >+ store.logged_in_library_id = loggedInLibraryId; >+ await this.loadUserPermissions(); > await this.getItemTypes(); > await this.getLibraries(); > await this.getPatronCategories(); >- this.currentLibraryId = defaultLibraryId; >+ // If user can only manage their own library, override the default view >+ if (!canManageAnyLibrary.value && store.logged_in_library_id) { >+ this.currentLibraryId = store.logged_in_library_id; >+ } else { >+ this.currentLibraryId = defaultLibraryId; >+ } > this.metaInitialized = true; > }, > async loadUserPermissions() { >@@ -515,5 +530,6 @@ export const useCircRulesStore = defineStore("circRules", () => { > return { > ...toRefs(store), > ...actions, >+ canManageAnyLibrary, > }; > }); >diff --git a/t/db_dependent/api/v1/circulation_rules.t b/t/db_dependent/api/v1/circulation_rules.t >index b1807160264..a30e1a6dc73 100755 >--- a/t/db_dependent/api/v1/circulation_rules.t >+++ b/t/db_dependent/api/v1/circulation_rules.t >@@ -299,7 +299,7 @@ subtest 'list_rules() tests' => sub { > }; > > subtest 'set_rules() tests' => sub { >- plan tests => 28; >+ plan tests => 34; > > $schema->storage->txn_begin; > >@@ -318,6 +318,18 @@ subtest 'set_rules() tests' => sub { > $librarian->set_password( { password => $password, skip_validation => 1 } ); > my $userid = $librarian->userid; > >+ # Grant manage_circ_rules_from_any_libraries so the librarian can set rules for any library >+ $builder->build( >+ { >+ source => 'UserPermission', >+ value => { >+ borrowernumber => $librarian->borrowernumber, >+ module_bit => 3, >+ code => 'manage_circ_rules_from_any_libraries', >+ } >+ } >+ ); >+ > my $patron = $builder->build_object( > { > class => 'Koha::Patrons', >@@ -328,6 +340,28 @@ subtest 'set_rules() tests' => sub { > $patron->set_password( { password => $password, skip_validation => 1 } ); > my $unauth_userid = $patron->userid; > >+ # Restricted librarian: has manage_circ_rules but NOT manage_circ_rules_from_any_libraries >+ my $own_branch = $builder->build( { source => 'Branch' } )->{'branchcode'}; >+ my $other_branch = $builder->build( { source => 'Branch' } )->{'branchcode'}; >+ my $restricted_patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0, branchcode => $own_branch } >+ } >+ ); >+ $restricted_patron->set_password( { password => $password, skip_validation => 1 } ); >+ my $restricted_userid = $restricted_patron->userid; >+ $builder->build( >+ { >+ source => 'UserPermission', >+ value => { >+ borrowernumber => $restricted_patron->borrowernumber, >+ module_bit => 3, >+ code => 'manage_circ_rules', >+ } >+ } >+ ); >+ > ## Authorized user tests > note("Authorized user setting rules"); > >@@ -416,5 +450,28 @@ subtest 'set_rules() tests' => sub { > { categorycode => undef, branchcode => undef, itemtype => undef, rule_name => 'finedays' } ); > is( $rules->count, 0, "Finedays rule deleted from database" ); > >+ # manage_circ_rules_from_any_libraries restriction tests >+ note("Testing manage_circ_rules_from_any_libraries restrictions"); >+ >+ my $restricted_rules = { >+ context => { >+ library_id => '*', >+ patron_category_id => '*', >+ item_type_id => '*', >+ }, >+ fine => 3, >+ }; >+ >+ $t->put_ok( "//$restricted_userid:$password@/api/v1/circulation_rules" => json => $restricted_rules ) >+ ->status_is( 403, "Restricted user cannot set rules for default (*) library" ); >+ >+ $restricted_rules->{context}->{library_id} = $other_branch; >+ $t->put_ok( "//$restricted_userid:$password@/api/v1/circulation_rules" => json => $restricted_rules ) >+ ->status_is( 403, "Restricted user cannot set rules for another library" ); >+ >+ $restricted_rules->{context}->{library_id} = $own_branch; >+ $t->put_ok( "//$restricted_userid:$password@/api/v1/circulation_rules" => json => $restricted_rules ) >+ ->status_is( 200, "Restricted user can set rules for their own library" ); >+ > $schema->storage->txn_rollback; > }; >-- >2.53.0 >
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 10190
:
170357
|
170358
|
170359
|
170360
|
170361
|
170362
|
170363
|
170364
|
170365
|
170366
|
170367
|
170368
|
170369
|
170370
|
170371
|
170372
|
170373
|
170374
|
170375
|
170376
|
170380
|
170449
|
171727
|
171728
|
171729
|
171730
|
171731
|
171732
|
171733
|
171734
|
171735
|
171736
|
171737
|
171738
|
171739
|
171740
|
171741
|
171742
|
171743
|
171744
|
171745
|
171746
|
171747
|
171748
|
171749
|
171750
|
171751
|
171752
|
171753
|
171754
|
171755
|
172588
|
172589
|
172590
|
172591
|
172592
|
172593
|
172594
|
172595
|
172596
|
172597
|
172598
|
172599
|
172600
|
172601
|
172602
|
172603
|
172604
|
172605
|
172606
|
172607
|
172608
|
172609
|
172612
|
172614
|
172617
|
172619
|
172621
|
172622
|
172623
|
172624
|
172625
|
172626
|
172630
|
172631
|
172632
|
172633
|
172634
|
172635
|
172636
|
172637
|
172638
|
172639
|
172640
|
172641
|
172642
|
172643
|
172644
|
172645
|
172646
|
172647
|
172648
|
172649
|
172650
|
172651
|
172652
|
172653
|
172654
|
172655
|
172656
|
172657
|
172658
|
172659
|
172660
|
172661
|
172662
|
172663
|
178643
|
178644
|
178645
|
178646
|
178647
|
178648
|
178649
|
178650
|
178651
|
178652
|
178653
|
178654
|
178655
|
178656
|
178657
|
178658
|
178659
|
178660
|
178661
|
178662
|
178663
|
178664
|
178665
|
178666
|
178667
|
178668
|
178669
|
178670
|
178671
|
178672
|
178673
|
178674
|
178675
|
188075
|
188076
|
188077
|
188078
|
188079
|
188080
|
188081
|
188082
|
188083
|
188084
|
188085
|
188086
|
188087
|
188088
|
188476
|
189345
|
189863
|
189864
|
189865
|
189866
|
189867
|
189868
|
191749
|
191750
|
191751
|
191752
|
191753
|
191754
|
191755
|
191756
|
191757
|
191758
|
191759
|
191760
|
191761
|
191762
|
191763
|
191795
|
191796
|
191805
|
191854
|
191873
|
192871
|
192872
|
192873
|
192874
|
192875
|
192876
|
192877
|
192878
|
192879
|
192880
|
192881
|
192882
|
192883
|
192884
|
192885
|
192886
|
192887
|
192888
|
192889
|
192890
|
192891
|
192892
|
192893
|
192894
|
192895
|
192896
|
192897
|
192898
|
192899
|
193864
|
193866
|
193867
|
193868
|
193869
|
193870
|
193871
|
193872
|
193873
|
193874
|
193875
|
193876
|
193877
|
193878
|
193879
|
193880
|
193881
|
193882
|
193883
|
193884
|
193885
|
193886
|
193887
|
193888
|
193889
|
193890
|
193891
|
193892
|
193893
|
193894
|
193895
|
193896
|
193897
|
193898
|
193899
|
193900
|
193901
|
193902
|
193903
|
193904
|
193905
|
193979
|
194045
|
194046
|
194047
|
194048
|
194049
|
194050
|
194051
|
194052
|
194053
|
194054
|
194055
|
194056
|
194057
|
194058
|
194059
|
194060
|
194061
|
194062
|
194063
|
194064
|
194065
|
194066
|
194067
|
194068
|
194069
|
194070
|
194071
|
194072
|
194073
|
194074
|
194075
|
194076
|
194077
|
194078
|
194079
|
194080
|
194081
|
194082
|
194083
|
194084
|
194085
|
194086
|
194087
| 194088 |
194090
|
194094