From 898e022b7b0b8b1bb184f0a6fb706609418f3d0e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 24 Feb 2026 10:27:58 +0000 Subject: [PATCH] Bug 33538: Update application to respect new sync fields Updates the Koha Object classes, API controller and definitions as well as the Auth handlers to understand the new sync fields. Test plan 1) Apply the full patchset 2) Update the sync definitions for an Identity provider 3) Login via that Identity Provider for the first time and confirm the prescribed fields synced. 4) Update some fields for your patron that correspond to both syned and unsynced fields on updates. 5) Login again via the Identity Provider and confirm unsynced fields remain unchanged and synced fields are updated back to the identity provider provided value. Sponsored-by: ByWater Solutions --- C4/Auth_with_shibboleth.pm | 12 +++++++---- Koha/Auth/Client.pm | 16 +++++++++++---- Koha/Auth/Client/OAuth.pm | 12 +++++++---- Koha/Auth/Identity/Provider/Mappings.pm | 6 ++++-- Koha/REST/Plugin/Auth/IdP.pm | 14 ++++++++++--- .../identity_provider_mapping.yaml | 8 ++++++++ .../IdentityProviders/MappingResource.vue | 20 +++++++++++++++++++ 7 files changed, 71 insertions(+), 17 deletions(-) diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm index 59e39382c00..ff22943d2e9 100644 --- a/C4/Auth_with_shibboleth.pm +++ b/C4/Auth_with_shibboleth.pm @@ -149,10 +149,11 @@ sub _autocreate { } while ( my ( $key, $entry ) = each %{ $config->{'mapping'} } ) { + next unless $entry->{'sync_on_creation'}; my $value = C4::Context->psgi_env - ? ( $entry->{'is'} && $ENV{ "HTTP_" . uc( $entry->{'is'} ) } ) || $entry->{'content'} || '' - : ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || ''; + ? ( $entry->{'is'} && $ENV{ "HTTP_" . uc( $entry->{'is'} ) } ) // $entry->{'content'} // '' + : ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) // $entry->{'content'} // ''; if ( $key =~ /^patron_attribute:(.+)$/ ) { $patron_attrs{$1} = $value; } else { @@ -213,10 +214,13 @@ sub _sync { my ( %borrower, %patron_attrs ); $borrower{'borrowernumber'} = $borrowernumber; while ( my ( $key, $entry ) = each %{ $config->{'mapping'} } ) { + next unless $entry->{'sync_on_update'}; my $value = C4::Context->psgi_env - ? ( $entry->{'is'} && $ENV{ "HTTP_" . uc( $entry->{'is'} ) } ) || $entry->{'content'} || '' - : ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) || $entry->{'content'} || ''; + ? ( $entry->{'is'} && $ENV{ "HTTP_" . uc( $entry->{'is'} ) } ) + : ( $entry->{'is'} && $ENV{ $entry->{'is'} } ); + next unless defined $value; + if ( $key =~ /^patron_attribute:(.+)$/ ) { $patron_attrs{$1} = $value; } else { diff --git a/Koha/Auth/Client.pm b/Koha/Auth/Client.pm index 370e2d9c0b2..c4acef0bac2 100644 --- a/Koha/Auth/Client.pm +++ b/Koha/Auth/Client.pm @@ -89,16 +89,24 @@ sub get_user { $patron = $args->{'patron'}; $domain = $args->{'domain'}; + my $mapping = $provider->mappings->as_auth_mapping; + if ( $patron && $domain->update_on_auth ) { my ( %patron_attrs, %borrower_data ); - for my $key ( keys %$mapped_data ) { + for my $key ( keys %$mapping ) { + next unless $mapping->{$key}->{sync_on_update}; + my $value = $mapped_data->{$key}; + + # We don't use default_content on update, only what we got from IdP + next unless defined $value; + if ( $key =~ /^patron_attribute:(.+)$/ ) { - $patron_attrs{$1} = $mapped_data->{$key}; + $patron_attrs{$1} = $value; } else { - $borrower_data{$key} = $mapped_data->{$key}; + $borrower_data{$key} = $value; } } - $patron->set( \%borrower_data )->store; + $patron->set( \%borrower_data )->store if %borrower_data; for my $code ( keys %patron_attrs ) { my $existing = Koha::Patron::Attributes->search( { borrowernumber => $patron->borrowernumber, code => $code } ); diff --git a/Koha/Auth/Client/OAuth.pm b/Koha/Auth/Client/OAuth.pm index d50911318ce..f5b653814db 100644 --- a/Koha/Auth/Client/OAuth.pm +++ b/Koha/Auth/Client/OAuth.pm @@ -74,9 +74,13 @@ sub _get_data_and_patron { my $claim = decode_json( decode_base64url($claims_part) ); foreach my $koha_field ( keys %$mapping ) { - my $pkey = $mapping->{$koha_field}{is}; - $mapped_data->{$koha_field} = $claim->{$pkey} - if defined $pkey && defined $claim->{$pkey}; + my $pkey = $mapping->{$koha_field}{is}; + my $value = defined $pkey ? $claim->{$pkey} : undef; + + # Note: We don't apply default here yet, because we need to know if it's a create or update. + # But we should at least store what we got. + $mapped_data->{$koha_field} = $value + if defined $value; } $patron = $self->_find_patron_by_matchpoint( $matchpoint, $mapped_data->{$matchpoint} ); @@ -96,7 +100,7 @@ sub _get_data_and_patron { foreach my $koha_field ( keys %$mapping ) { my $pkey = $mapping->{$koha_field}{is}; - my $value = $self->_traverse_hash( { base => $claim, keys => $pkey } ); + my $value = defined $pkey ? $self->_traverse_hash( { base => $claim, keys => $pkey } ) : undef; $mapped_data->{$koha_field} = $value if defined $value; } diff --git a/Koha/Auth/Identity/Provider/Mappings.pm b/Koha/Auth/Identity/Provider/Mappings.pm index 2e97223f7ab..eef464c7a0a 100644 --- a/Koha/Auth/Identity/Provider/Mappings.pm +++ b/Koha/Auth/Identity/Provider/Mappings.pm @@ -46,8 +46,10 @@ sub as_auth_mapping { while ( my $m = $self->next ) { $mapping{ $m->koha_field } = { - is => $m->provider_field, - content => $m->default_content, + is => $m->provider_field, + content => $m->default_content, + sync_on_creation => $m->sync_on_creation, + sync_on_update => $m->sync_on_update, }; } diff --git a/Koha/REST/Plugin/Auth/IdP.pm b/Koha/REST/Plugin/Auth/IdP.pm index 2f60bf6e36d..baae598ca59 100644 --- a/Koha/REST/Plugin/Auth/IdP.pm +++ b/Koha/REST/Plugin/Auth/IdP.pm @@ -90,12 +90,20 @@ if other values or none are passed. Koha::Exceptions::Auth::Unauthorized->throw( code => 401 ); } + my $provider = $domain->identity_provider; + my $mapping = $provider->mappings->as_auth_mapping; + my ( %patron_attrs, %borrower_data ); - for my $key ( keys %$data ) { + for my $key ( keys %$mapping ) { + next unless $mapping->{$key}->{sync_on_creation}; + my $value = $data->{$key}; + $value //= $mapping->{$key}->{content}; + next unless defined $value; + if ( $key =~ /^patron_attribute:(.+)$/ ) { - $patron_attrs{$1} = $data->{$key}; + $patron_attrs{$1} = $value; } else { - $borrower_data{$key} = $data->{$key}; + $borrower_data{$key} = $value; } } my $patron = Koha::Patron->new( \%borrower_data )->store; diff --git a/api/v1/swagger/definitions/identity_provider_mapping.yaml b/api/v1/swagger/definitions/identity_provider_mapping.yaml index c1f7f0e52b0..9570722c68e 100644 --- a/api/v1/swagger/definitions/identity_provider_mapping.yaml +++ b/api/v1/swagger/definitions/identity_provider_mapping.yaml @@ -22,6 +22,14 @@ properties: - string - "null" description: Default value to use if the provider does not supply this attribute + sync_on_creation: + type: boolean + description: Whether to sync this field on user creation + default: true + sync_on_update: + type: boolean + description: Whether to sync this field on user update + default: true additionalProperties: false required: - koha_field diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/IdentityProviders/MappingResource.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/IdentityProviders/MappingResource.vue index 97eb89012c3..00187d54ea9 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/IdentityProviders/MappingResource.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/IdentityProviders/MappingResource.vue @@ -61,6 +61,26 @@ export default { "Value to use when the provider does not supply this attribute" ), }, + { + name: "sync_on_creation", + type: "checkbox", + label: __("Sync on creation"), + group: "Mapping", + toolTip: __( + "If checked, this field will be synced when the user is first created" + ), + default: true, + }, + { + name: "sync_on_update", + type: "checkbox", + label: __("Sync on update"), + group: "Mapping", + toolTip: __( + "If checked, this field will be synced on subsequent logins" + ), + default: true, + }, ]; const baseResource = useBaseResource({ -- 2.53.0