gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Signed-off-by: Michaela <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-02-27 07:06:24 +00:00 committed by Pedro Amorim
parent 14fe8cb2f5
commit 88b16e6445
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
9 changed files with 147 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
Koha/REST/V1/CirculationRules.pm
View File

@ -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(

3
api/v1/swagger/paths/circulation_rules.yaml
View File

@ -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

1
api/v1/swagger/paths/circulation_rules_config.yaml
View File

@ -36,3 +36,4 @@
x-koha-authorization:
permissions:
- circulate: circulate_remaining_permissions
- parameters: manage_circ_rules

1
koha-tmpl/intranet-tmpl/prog/en/modules/admin/circulation_triggers.tt
View File

@ -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 %]

25
koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue
View File

@ -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 ?? "*";
},

35
koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersList.vue
View File

@ -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();

4
koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/Main.vue
View File

@ -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 {

22
koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js
View File

@ -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,
};
});

59
t/db_dependent/api/v1/circulation_rules.t
View File

@ -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;
};