From 746d50d17ed0c971a46122807710e0fdc430edaf 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 | 73 +++++++++--------- .../prog/en/modules/members/moremember.tt | 10 ++- members/memberentry.pl | 38 +++++++--- members/moremember.pl | 16 ++-- 12 files changed, 300 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 fd7cf85fc21..8e56797d39d 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2270,14 +2270,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; @@ -2332,6 +2349,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 d9055483d21..372eb0778f2 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 75caa255d00..6eb805bbefb 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 f7e6e4daa90..058e418563e 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 f31cef713b2..943706817e6 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 %] @@ -424,6 +451,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(); $("#opac_display") .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 f94d479da5e..7d03122bad0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1512,6 +1512,10 @@
    + [% FOREACH patron_attribute IN hidden_patron_attributes %] + + + [% END %] [% FOREACH pa_loo IN patron_attributes %]
      @@ -1519,14 +1523,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 %] @@ -1536,45 +1548,38 @@ [% 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 16c2b333803..cca300ef5c1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -350,10 +350,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 0794a9cff34..6f7f3698448 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 } @@ -627,7 +629,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 ( @@ -927,7 +929,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; } @@ -941,12 +945,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(), @@ -957,15 +967,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++; @@ -988,7 +1002,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 d08cef77a0e..b0653a7eb47 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 ); @@ -138,15 +139,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; @@ -161,9 +163,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.49.0