Problem/Motivation

In current code, user_name_attribute & user_mail_attribute have default values, but are not mandatory. If site admin save empty value in one of these fields, next time he/she will load Samlauth admin page, the field will be filled again with default value. So if site admin saves the form (while editing other settings for example), the default value will be back in config.

Proposed resolution

Just remove the default values for these fields (and maybe add more examples in field description).
Attached patch just does that.

CommentFileSizeAuthor
#4 FireShot Capture 142 - SAML Authentication - Drush Site-Install_ - stm5e6f84ebb331f-wkrvdorpi6onkhefyefwspa8zfa7eawu.tugboat.qa_.png349.83 KBKondratievaS
remove-default-values.patch1.63 KBnguerrier

Comments

nguerrier created an issue. See original summary.

nguerrier’s picture

nguerrier commented
Status: Active » Needs review
nguerrier’s picture

nguerrier commented
Related issues: +#3087233: Ignore e-mail synchronization if user_mail_attribute is empty

Related issue #3087233: Ignore e-mail synchronization if user_mail_attribute is empty

KondratievaS’s picture

KondratievaS commented
Issue summary: View changes
StatusFileSize
new FireShot Capture 142 - SAML Authentication - Drush Site-Install_ - stm5e6f84ebb331f-wkrvdorpi6onkhefyefwspa8zfa7eawu.tugboat.qa_.png349.83 KB

Tested provided patch for 8.x-3.x-dev version of module and result is OK - field can be empty now

KondratievaS’s picture

KondratievaS commented
Status: Needs review » Reviewed & tested by the community

  • roderik committed 5b52f82 on 8.x-3.x authored by nguerrier 
    Issue #3103165 by nguerrier, KondratievaS: Remove default value for user...
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Reviewed & tested by the community » Fixed

Thank you. This was indeed an oversight.

If we were attached to having a default value, we could replace the ?: by ?? instead of removing the default. But I'm not attached to it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.