Bug and confusing behaviour when trying to set a custom NameID format which is already known to the module

Open on Drupal.org →

Created on 29 August 2024, 3 months ago

Hello all,

I have been setting up this module with a custom NameID of "urn:oasis:names:tc:SAML:2.0:nameid-format:transient":

However, every time I saved the form, one of two things was occurring; either:

1) the NameID format was reset to "Transient" and, because nowhere in the interface is the actual value of "Transient" shown, I didn't understand why my selection of "Other" wasn't persisting after save, or:

2) The "Custom NameID" option was still selected, but the custom NameID format field contained an asterisk:

Those of you familiar with this module will know that, when building the form, if a custom NameID format is entered which is already known to the module, the code in samlauth/src/Form/SamlauthConfigureTrait.php in function addNameID(array &$build, array $schema_definition, Config $config) tries to select the already-known NameID format and should clear the custom NameID field.

However, there seems to be a bug which causes the custom NameID field's value to be set to *, and the user interface doesn't explicitly list the various NameID formats.

I have coded up a fix which:

1. Adds the full NameID format string into the drop-down select options, after the "User-friendly" name, so administrators can see the exact format name when configuring NameID, and

2. Fixes the problem where the custom NameID field's value is being incorrectly set to "*".

One moment while I figure out how to do issue forks...

/Alex

🐛 Bug report

Status

Needs review

Version

3.10

Component

Code

Created by

🇬🇧United Kingdom alexharries

Live updates comments and jobs are added and updated live.

Sign in to follow issues

Merge Requests

!22Bug and confusing behaviour when trying to set a custom NameID format which is already known to the module
Open
🇬🇧United Kingdom alexharries
updated 3 months ago

Comments & Activities

Issue created by @alexharries
Comment 3 months ago →
🇬🇧United Kingdom alexharries
Patch attached for reference:

https://www.drupal.org/files/issues/2024-08-29/DD-1794-samlauth--Fix-con... →
Status changed to Needs review 3 months ago2:46pm 29 August 2024
Comment 3 months ago →
🇬🇧United Kingdom alexharries
Merge request !22Fix confusing custom NameID UX → (Open) created by alexharries
Pipeline finished with Success
3 months ago
Total: 290s
#268352
Comment 3 months ago →
🇬🇧United Kingdom alexharries
Hmm, there's some magic going on here I wasn't aware of; the sp_name_id_format field is actually used to store custom formats, too; we only populate sp_name_id_format_ for the form.

When saving the NameID format, sp_name_id_format_ isn't saved into config.

Re-rolling my code/a patch to handle this correctly gives the attached.

/A
Comment 3 months ago →
🇬🇧United Kingdom alexharries
Pipeline finished with Success
3 months ago
Total: 256s
#268388
Comment 3 months ago →
🇬🇧United Kingdom alexharries
One last patch from me; I found that submitForm() in SamlauthSamlConfigureForm.php was overwriting the value of sp_name_id_format; when a custom NameID format is set, setNameId() in src/Form/SamlauthConfigureTrait.php correctly sets it to the custom value, but then this function overwrites the custom value with the asterisk from the drop-down NameID formats selector.

{facepalm.gif}

Patch attached and pushed to the MR.

/A

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024