Bugzilla – Attachment 193703 Details for
Bug 33538
Allow for independent shibboleth autocreate and sync mapping
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33538: Update application to respect new sync fields
898e022.patch (text/plain), 9.61 KB, created by
Martin Renvoize (ashimema)
on 2026-02-24 10:34:44 UTC
(
hide
)
Description:
Bug 33538: Update application to respect new sync fields
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-24 10:34:44 UTC
Size:
9.61 KB
patch
obsolete
>From 898e022b7b0b8b1bb184f0a6fb706609418f3d0e Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33538
:
193700
|
193701
|
193702
| 193703 |
193704