From a0317dfa33a0fdc0cf95d007ae901bcae6613e4e 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 | 11 +++ Koha/Patron.pm | 25 +++++- Koha/Patron/Attribute.pm | 2 + Koha/Patron/Attribute/Type.pm | 68 +++++++++++++++++ admin/patron-attr-types.pl | 6 ++ ...new-columns-to-borrower-attribute-types.pl | 16 ++++ .../en/modules/admin/patron-attr-types.tt | 76 +++++++++++++++++++ .../prog/en/modules/members/memberentrygen.tt | 59 ++++++-------- .../prog/en/modules/members/moremember.tt | 10 ++- members/memberentry.pl | 34 ++++++--- members/moremember.pl | 10 ++- 12 files changed, 273 insertions(+), 54 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..5e02415fc01 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 { @@ -28,6 +33,12 @@ sub full_message { $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 + ); + } } return $msg; diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fd7cf85fc21..8a17d920248 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2270,14 +2270,29 @@ 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. Allways 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. + =cut sub extended_attributes { - my ( $self, $attributes ) = @_; + my ( $self, $attributes, $params ) = @_; + $params //= {}; if ($attributes) { # setter my %attribute_changes; @@ -2332,6 +2347,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..4969c7ee7c1 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,71 @@ 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..184f38d5879 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_39453-add-new-columns-to-borrower-attribute-types.pl @@ -0,0 +1,16 @@ +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..6d106289930 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..a1f9343c5f5 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,14 @@

      [% 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 +1540,32 @@ [% 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..1d54c666386 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -24,6 +24,7 @@ use Try::Tiny; # external modules 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,7 @@ 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 +943,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 +965,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 +1000,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..7ebfcd230a4 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -28,6 +28,7 @@ use Modern::Perl; 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,6 +139,7 @@ unless ( Koha::Patron::Categories->search_with_library_limits( { 'me.categorycod } if ( C4::Context->preference('ExtendedPatronAttributes') ) { + 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; @@ -146,7 +148,7 @@ if ( C4::Context->preference('ExtendedPatronAttributes') ) { 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.48.1