From ce69f0309e2cd2e8f19a973a43c1e5cfc251b5e5 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Tue, 25 Mar 2025 21:28:12 +0100 Subject: [PATCH] Bug 39453: Add attribute type settings for restricting access to extended attributes Add new attribute type settings "readonly", "secret" and "hidden" for patron attribute types. All restrictions enforced by these new settings are not applied for superlibrarians, where attributes behave as if they have not been set. Readonly make the attributes of this type readonly, they appear on the patrons details page and when editing a patron, but their values can't be changed. Hidden attributes are not visible on the patrons details page or when editing a patron, but can for example be used internally. Secret attributes are basically the same thing as hidden, but the appear with their value masked in the patrons details page. To Test: 1) Apply patch 3) Three new attribute type, each with one of the new settings set. 4) In addition to this create an attribute type which is a date, and set this to readonly. 5) As a superlibrarian, edit a patron and set values for the attributes of these types6) Log in as a non superlibrarian. Go to the same patron and verify that: 7) The attributes that are readonly can be viewd in the patrons details page, and when editing the patron, but their value can't be changed. 8) The hidden attribute should appear nowhere. 9) The secret attribute should be visible in the patrons details page, but masked. It should not appear when editing the patron. 10) Edit the patrons extended patron and save. 11) Log in as a superlibrarian, go to the patrons details page and verify the extended attribute values remains the same. Sponsored-by: Gothenburg University Library --- Koha/Exceptions/Patron/Attribute.pm | 10 +++ Koha/Exceptions/Patron/Attribute/Type.pm | 10 +++ Koha/Patron.pm | 27 ++++++- Koha/Patron/Attribute.pm | 2 + Koha/Patron/Attribute/Type.pm | 71 +++++++++++++++++ admin/patron-attr-types.pl | 6 ++ ...new-columns-to-borrower-attribute-types.pl | 19 +++++ .../en/modules/admin/patron-attr-types.tt | 76 +++++++++++++++++++ .../prog/en/modules/members/memberentrygen.tt | 74 +++++++++--------- .../prog/en/modules/members/moremember.tt | 10 ++- members/memberentry.pl | 38 +++++++--- members/moremember.pl | 16 ++-- 12 files changed, 301 insertions(+), 58 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_39453-add-new-columns-to-borrower-attribute-types.pl diff --git a/Koha/Exceptions/Patron/Attribute.pm b/Koha/Exceptions/Patron/Attribute.pm index f513d093bfd..206dbce70ad 100644 --- a/Koha/Exceptions/Patron/Attribute.pm +++ b/Koha/Exceptions/Patron/Attribute.pm @@ -28,6 +28,11 @@ use Exception::Class ( isa => 'Koha::Exceptions::Patron::Attribute', description => "the passed value is invalid for attribute type", fields => ["attribute"] + }, + 'Koha::Exceptions::Patron::Attribute::NonEditable' => { + isa => 'Koha::Exceptions::Patron::Attribute', + description => "Cannot to modify value of non editable attribute type", + fields => ["type"] } ); @@ -60,6 +65,11 @@ sub full_message { $self->attribute->code, $self->attribute->attribute ); + } elsif ( $self->isa('Koha::Exceptions::Patron::Attribute::NonEditable') ) { + $msg = sprintf( + "Cannot change value pf non editable attribute type. type=%s", + $self->type + ); } } diff --git a/Koha/Exceptions/Patron/Attribute/Type.pm b/Koha/Exceptions/Patron/Attribute/Type.pm index 9decc8cfc5f..9e302368ac4 100644 --- a/Koha/Exceptions/Patron/Attribute/Type.pm +++ b/Koha/Exceptions/Patron/Attribute/Type.pm @@ -14,6 +14,11 @@ use Exception::Class ( description => "Cannot change property", fields => ['property'], }, + 'Koha::Exceptions::Patron::Attribute::Type::InvalidPropertyCombination' => { + isa => 'Koha::Exceptions::Patron::Attribute::Type', + description => "An invalid property combination has been set", + fields => ['property'], + }, ); sub full_message { @@ -27,6 +32,11 @@ sub full_message { "The property '%s' cannot be changed, some patron attributes are using it that way.", $self->property ); + } elsif ( $self->isa('Koha::Exceptions::Patron::Attribute::Type::InvalidPropertyCombination') ) { + $msg = sprintf( + "The property '%s' is incompatible with the current combination of properties", + $self->property + ); } } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 03276ff6a54..2230c7f0733 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2607,14 +2607,31 @@ sub add_extended_attribute { =head3 extended_attributes -Return object of Koha::Patron::Attributes type with all attributes set for this patron +Getter/setter for Koha::Patron::Attributes. Always return the extended attributes of the patron. Or setter FIXME +=over 4 + +=item C<$extended_attributes> + +The extended attributes to set on the format [{ code => $code, attribute => $value }, ...]. +All extended attributes of the patron must be provided. + +=item C<$params> + +A hashfre for Optional parameters. + +if called from within the staff interface C must be set so that +attribute type settings making the attribute non editable are enforced. + +=back + =cut sub extended_attributes { - my ( $self, $attributes ) = @_; + my ( $self, $attributes, $params ) = @_; + $params //= {}; if ($attributes) { # setter my %attribute_changes; @@ -2669,6 +2686,12 @@ sub extended_attributes { $change->{after} //= []; if ( $is_different->( $change->{before}, $change->{after} ) ) { + if ( $params->{check_editable} ) { + my $type = Koha::Patron::Attribute::Types->find($code); + unless ( $type->is_editable ) { + Koha::Exceptions::Patron::Attribute::NonEditable->throw( type => $code ); + } + } $changed_attributes_codes{$code} = 1; unless ($repeatable) { diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index 7728dc1a24a..3eec5d900b2 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -20,8 +20,10 @@ use Modern::Perl; use Koha::Database; use Koha::Exceptions::Patron::Attribute; use Koha::Patron::Attribute::Types; +use Koha::Patron::Attributes; use Koha::AuthorisedValues; use Koha::DateUtils qw( dt_from_string ); +use C4::Context; use base qw(Koha::Object); diff --git a/Koha/Patron/Attribute/Type.pm b/Koha/Patron/Attribute/Type.pm index 31218020e99..b9851a3b687 100644 --- a/Koha/Patron/Attribute/Type.pm +++ b/Koha/Patron/Attribute/Type.pm @@ -46,6 +46,9 @@ sub store { $self->check_repeatables; $self->check_unique_ids; + $self->check_hidden; + $self->check_readonly; + $self->check_secret; return $self->SUPER::store(); } @@ -112,6 +115,74 @@ sub check_unique_ids { return $self; } +=head3 check_hidden + +=cut + +sub check_hidden { + my ($self) = @_; + if ( + $self->hidden + && ( $self->readonly + || $self->secret + || $self->opac_display + || $self->opac_editable + || $self->display_checkout ) + ) + { + Koha::Exceptions::Patron::Attribute::Type::InvalidPropertyCombination->throw( property => 'hidden' ); + } +} + +=head3 check_readonly + +=cut + +sub check_readonly { + my ($self) = @_; + if ( + $self->readonly + && ( $self->hidden + || $self->secret + || $self->opac_editable ) + ) + { + Koha::Exceptions::Patron::Attribute::Type::InvalidPropertyCombination->throw( property => 'readonly' ); + } +} + +=head3 check_secret + +=cut + +sub check_secret { + my ($self) = @_; + if ( + $self->secret + && ( $self->readonly + || $self->hidden + || $self->opac_display + || $self->opac_editable + || $self->display_checkout ) + ) + { + Koha::Exceptions::Patron::Attribute::Type::InvalidPropertyCombination->throw( property => 'hidden' ); + } +} + +=head3 is_editable + +=cut + +sub is_editable { + my ($self) = @_; + my $logged_in_borrowernumber = C4::Context->userenv->{'number'}; + if ($logged_in_borrowernumber) { + my $patron = Koha::Patrons->find($logged_in_borrowernumber); + return $patron->_is_superlibrarian && !( $self->hidden || $self->readonly || $self->secret ) if $patron; + } +} + =head3 _type =cut diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl index 47c16306a8a..799b93ef6cc 100755 --- a/admin/patron-attr-types.pl +++ b/admin/patron-attr-types.pl @@ -109,6 +109,9 @@ sub add_update_attribute_type { my $description = $input->param('description'); my $repeatable = $input->param('repeatable') ? 1 : 0; my $unique_id = $input->param('unique_id') ? 1 : 0; + my $hidden = $input->param('hidden') ? 1 : 0; + my $readonly = $input->param('readonly') ? 1 : 0; + my $secret = $input->param('secret') ? 1 : 0; my $is_date = $input->param('is_date') ? 1 : 0; my $opac_display = $input->param('opac_display') ? 1 : 0; my $opac_editable = $input->param('opac_editable') ? 1 : 0; @@ -146,6 +149,9 @@ sub add_update_attribute_type { { repeatable => $repeatable, unique_id => $unique_id, + hidden => $hidden, + readonly => $readonly, + secret => $secret, is_date => $is_date, opac_display => $opac_display, opac_editable => $opac_editable, diff --git a/installer/data/mysql/atomicupdate/bug_39453-add-new-columns-to-borrower-attribute-types.pl b/installer/data/mysql/atomicupdate/bug_39453-add-new-columns-to-borrower-attribute-types.pl new file mode 100755 index 00000000000..9d846da2cc0 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_39453-add-new-columns-to-borrower-attribute-types.pl @@ -0,0 +1,19 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_success say_info); + +return { + bug_number => "39453", + description => "Add attribute type settings for restricting access to extended attributes", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do( + q{ALTER TABLE `borrower_attribute_types` ADD COLUMN `hidden` tinyint(1) NOT NULL DEFAULT 0, ADD COLUMN `readonly` tinyint(1) NOT NULL DEFAULT 0, ADD COLUMN `secret` tinyint(1) NOT NULL DEFAULT 0} + ); + + say $out + "Added columns 'borrower_attribute_types.hidden', 'borrower_attribute_types.readonly' and 'borrower_attribute_types.secret'"; + + }, +}; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt index 631a41867b8..34605122bb4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt @@ -139,6 +139,33 @@ [% END %] If checked, attribute will be a unique identifier. If a value is given to a patron record, the same value cannot be given to a different record. +
  • + + [% IF attribute_type AND attribute_type.hidden %] + + [% ELSE %] + + [% END %] + Check to hide this attribute on a patron's details page in the OPAC and staff interface, and when creating or editing a patron from the staff interface. Does not apply for superlibrarians. +
  • +
  • + + [% IF attribute_type AND attribute_type.readonly %] + + [% ELSE %] + + [% END %] + Check to make this attribute readonly when creating or editing a patron from the staff interface. Does not apply for superlibrarians. +
  • +
  • + + [% IF attribute_type AND attribute_type.secret %] + + [% ELSE %] + + [% END %] + Check to mask attribute value on a patron's details page, and hide when creating or editing a patron. Does not apply for superlibrarians. +
  • [% IF attribute_type AND attribute_type.is_date %] @@ -459,6 +486,55 @@ if ($("#branches option:selected").length < 1) { $("#branches option:first").attr("selected", "selected"); } + $("#hidden") + .change(function () { + if (this.checked) { + $("#opac_editable").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#opac_display").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#readonly").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#secret").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#display_checkout").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + } else { + $("#opac_editable").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#opac_display").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#readonly").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#secret").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#display_checkout").removeAttr("disabled").parent().removeAttr("aria-disabled"); + } + }) + .change(); + + $("#readonly") + .change(function () { + if (this.checked) { + $("#opac_editable").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#hidden").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#secret").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + } else { + $("#opac_editable").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#hidden").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#secret").removeAttr("disabled").parent().removeAttr("aria-disabled"); + } + }) + .change(); + + $("#secret") + .change(function () { + if (this.checked) { + $("#opac_editable").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#opac_display").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#hidden").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#readony").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + $("#display_checkout").attr("disabled", true).prop("checked", false).parent().attr("aria-disabled", "true"); + } else { + $("#opac_editable").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#opac_display").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#hidden").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#readonly").removeAttr("disabled").parent().removeAttr("aria-disabled"); + $("#display_checkout").removeAttr("disabled").parent().removeAttr("aria-disabled"); + } + }) + .change(); $("#is_date") .change(function () { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index db1ad64809e..698b8c45154 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1515,6 +1515,10 @@
    + [% FOREACH patron_attribute IN hidden_patron_attributes %] + + + [% END %] [% FOREACH pa_loo IN patron_attributes %]
      @@ -1522,14 +1526,22 @@

      [% pa_loo.lib | html %]

      [% END %] [% FOREACH patron_attribute IN pa_loo.items %] -
    1. +
    2. [% IF patron_attribute.mandatory %] [% ELSE %] [% END %] [% IF ( patron_attribute.use_dropdown ) %] - [% FOREACH auth_val_loo IN patron_attribute.auth_val_loop %] [% IF auth_val_loo.authorised_value == patron_attribute.value %] @@ -1539,45 +1551,39 @@ [% END %] [% END %] + [% IF ( patron_attribute.readonly ) %] + + [% END %] [% ELSE %] - [% IF patron_attribute.mandatory %] - [% IF patron_attribute.is_date %] - - [% ELSE %] - - [% END %] + [% IF patron_attribute.is_date %] + [% ELSE %] - [% IF patron_attribute.is_date %] - - [% ELSE %] - - [% END %] + [% END %] [% END # /IF ( patron_attribute.use_dropdown ) %] - [% IF ( !patron_attribute.is_date ) %] + [% IF ( !patron_attribute.is_date && !patron_attribute.readonly ) %] Clear [% END %] - [% IF ( patron_attribute.repeatable ) %] + [% IF ( patron_attribute.repeatable && !patron_attribute.readonly ) %] New [% END %] [% IF patron_attribute.mandatory %]Required[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index 6ac0d5fd485..48291f5bad3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -359,10 +359,14 @@ [% FOREACH item IN attribute.items %]
    3. [% item.type.description | html %]: - [% IF item.type.is_date %] - [% item.description | $KohaDates %] + [% IF item.type.secret %] + ***** [% ELSE %] - [% item.description | html_line_break %] + [% IF item.type.is_date %] + [% item.description | $KohaDates %] + [% ELSE %] + [% item.description | html_line_break %] + [% END %] [% END %]
    4. [% END %] diff --git a/members/memberentry.pl b/members/memberentry.pl index 50343bcc04d..67db58cb36b 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -23,7 +23,8 @@ use Modern::Perl; use Try::Tiny; # external modules -use CGI qw ( -utf8 ); +use CGI qw ( -utf8 ); +use List::Util qw ( all ); # internal modules use C4::Auth qw( get_template_and_user haspermission ); @@ -164,8 +165,9 @@ foreach (@field_check) { $template->param( "quickadd" => 1 ) if ($quickadd); $template->param( "duplicate" => 1 ) if ( $op eq 'duplicate' ); $template->param( "checked" => 1 ) if ( defined($nodouble) && $nodouble eq 1 ); +my $logged_in_user = Koha::Patrons->find($loggedinuser); + if ( $op eq 'edit_form' or $op eq 'cud-save' or $op eq 'duplicate' ) { - 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 } @@ -632,7 +634,7 @@ if ( ( !$nok ) and $nodouble and ( $op eq 'cud-insert' or $op eq 'cud-save' ) ) if ( C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes') ) { - $patron->extended_attributes($extended_patron_attributes); + $patron->extended_attributes( $extended_patron_attributes, { check_editable => 1 } ); } if ( @@ -936,7 +938,9 @@ sub patron_attributes_form { my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; my $attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits( {}, {}, $library_id ); - if ( $attribute_types->count == 0 ) { + if ( $attribute_types->count == 0 + || ( !$logged_in_user->is_superlibrarian && all { $_->hidden } $attribute_types->as_list ) ) + { $template->param( no_patron_attribute_types => 1 ); return; } @@ -950,12 +954,18 @@ sub patron_attributes_form { my @attribute_loop = (); my $i = 0; my %items_by_class; + my @hidden_attributes; + + my $is_superlibrarian = $logged_in_user->is_superlibrarian; while ( my ($attr_type) = $attribute_types->next ) { my $entry = { class => $attr_type->class(), code => $attr_type->code(), description => $attr_type->description(), repeatable => $attr_type->repeatable(), + hidden => !$is_superlibrarian && $attr_type->hidden(), + readonly => !$is_superlibrarian && $attr_type->readonly(), + secret => !$is_superlibrarian && $attr_type->secret(), category => $attr_type->authorised_value_category(), category_code => $attr_type->category_code(), mandatory => $attr_type->mandatory(), @@ -966,15 +976,19 @@ sub patron_attributes_form { my $newentry = {%$entry}; $newentry->{value} = $attr->{attribute}; $newentry->{use_dropdown} = 0; - if ( $attr_type->authorised_value_category() ) { - $newentry->{use_dropdown} = 1; - $newentry->{auth_val_loop} = - GetAuthorisedValues( $attr_type->authorised_value_category(), $attr->{attribute} ); - } $i++; - undef $newentry->{value} if ( $attr_type->unique_id() && $op eq 'duplicate' ); $newentry->{form_id} = "patron_attr_$i"; - push @{ $items_by_class{ $attr_type->class() } }, $newentry; + if ( !$is_superlibrarian && ( $attr_type->hidden() || $attr_type->secret() ) ) { + push @hidden_attributes, $newentry; + } else { + if ( $attr_type->authorised_value_category() ) { + $newentry->{use_dropdown} = 1; + $newentry->{auth_val_loop} = + GetAuthorisedValues( $attr_type->authorised_value_category(), $attr->{attribute} ); + } + undef $newentry->{value} if ( $attr_type->unique_id() && $op eq 'duplicate' ); + push @{ $items_by_class{ $attr_type->class() } }, $newentry; + } } } else { $i++; @@ -997,7 +1011,7 @@ sub patron_attributes_form { }; } - $template->param( patron_attributes => \@attribute_loop ); + $template->param( patron_attributes => \@attribute_loop, hidden_patron_attributes => \@hidden_attributes ); } diff --git a/members/moremember.pl b/members/moremember.pl index 9cf8574e101..6f3d8506690 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -27,7 +27,8 @@ =cut use Modern::Perl; -use CGI qw ( -utf8 ); +use CGI qw ( -utf8 ); +use List::Util qw ( all ); use C4::Context; use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_and_exit_if_error output_and_exit output_html_with_http_headers ); @@ -142,15 +143,16 @@ unless ( Koha::Patron::Categories->search_with_library_limits( { 'me.categorycod } if ( C4::Context->preference('ExtendedPatronAttributes') ) { - my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved! - my @classes = uniq( map { $_->type->class } @attributes ); + my $is_superlibrarian = $logged_in_user->is_superlibrarian; + my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved! + my @classes = uniq( map { $_->type->class } @attributes ); @classes = sort @classes; my @attributes_loop; for my $class (@classes) { my @items; for my $attr (@attributes) { - push @items, $attr if $attr->type->class eq $class; + push @items, $attr if $attr->type->class eq $class && ( $is_superlibrarian || !$attr->hidden ); } my $av = Koha::AuthorisedValues->search( { category => 'PA_CLASS', authorised_value => $class } ); my $lib = $av->count ? $av->next->lib : $class; @@ -165,9 +167,9 @@ if ( C4::Context->preference('ExtendedPatronAttributes') ) { $template->param( attributes_loop => \@attributes_loop ); my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; - my $nb_of_attribute_types = - Koha::Patron::Attribute::Types->search_with_library_limits( {}, {}, $library_id )->count; - if ( $nb_of_attribute_types == 0 ) { + my $attribute_types = + Koha::Patron::Attribute::Types->search_with_library_limits( {}, {}, $library_id ); + if ( $attribute_types->count == 0 || ( !$is_superlibrarian && all { $_->hidden } $attribute_types->as_list ) ) { $template->param( no_patron_attribute_types => 1 ); } } -- 2.51.2