feat: #3522489 Synchronize multi-value SAML attributes into multi-value user fields.

What this does

samlauth_user_fields currently writes only the first value of a SAML attribute, even when the mapped user field can hold several. With this change, if the destination field's storage allows more than one value (isMultiple()), all values of the attribute are synchronized into it, as you suggested in #5.

  • Empty-string values are skipped. An attribute with no non-empty values leaves the field untouched (never cleared), same as the single-value path.
  • Values beyond the field's cardinality are dropped, with a debug log message, instead of failing validation for the whole list.
  • If several attributes are mapped to the same multi-value field, their values are combined in mapping order.
  • The comparison is order-sensitive. The account is only marked changed if the value list actually differs.
  • The new list is validated with the existing validateAccountFieldValue() before it's set.
  • The current value is read from the field's main property, so field types whose main property isn't value (e.g. link → uri) compare correctly.

Unchanged: single-value fields (including name/mail), compound sub-fields such as field_address:locality, and user linking (onUserLink()) all still use the first value.

Implementation notes

  • onUserSync(): field-name parsing moved before reading the attribute, because the cardinality decides how the attribute is read. Multi-value fields are collected during the loop and written afterwards, alongside the existing compound-field step.
  • New protected helpers: getUpdatableAttributeValues() (list equivalent of getUpdatableAttributeValue()) and areInputValuesEqual().
  • Small fix: invalid mappings (empty or non-string attribute_name/field_name) now continue after the warning, instead of being processed anyway.
  • README "Known-incomplete functionality" section, CHANGELOG entry and the mapping form's field description are updated.

Behavior change for existing sites

A site that already maps an attribute to a multi-value field will see that field mirror the IdP's full list. Today that field is already reduced to a single value whenever the attribute's first value changes, so the difference is mainly that additional values are now kept (and kept in sync).

Questions

  1. Opt-in? If you'd rather not change existing behavior, I can add a per-mapping "Synchronize all values" checkbox: missing key means off, so existing mappings are unaffected and no update hook is needed, and it defaults to on for new mappings. It's about 30 lines; happy to push it to this MR.
  2. Subclasses: a subclass overriding getAttribute() / getUpdatableAttributeValue() won't affect multi-value fields anymore, since those go through getUpdatableAttributeValues(). Fine, or should the new method delegate somehow?
  3. Duplicates: values aren't deduplicated, so mapping both an attribute's Name and its FriendlyName to the same field doubles the values. I left that alone because deduplicating would also collapse duplicates the IdP sends deliberately.

Testing

  • New kernel test tests/src/Kernel/UserFieldsSyncTest.php (10 cases: single vs. multi-value, empty values, replacement, cardinality truncation, no-change detection, ordering, combining attributes). It passes locally on Drupal 11.4 / PHP 8.3, and the existing AcsUserSyncTest still passes.
  • Manual: full SAML login against a SimpleSAMLphp test IdP, mapping a three-value attribute to an unlimited field and to a cardinality-2 field. Results matched the above.
  • Not tested locally on Drupal 10; relying on CI for that.

By: kfolsom Developed with AI assistance (Claude); kfolsom reviewed and tested the changes.

Closes #3522489

Merge request reports

Loading
Loading