GluuFederation / oxTrust

Gluu Server UI for managing authentication, authorization and users.
https://gluu.org/docs/ce
MIT License
134 stars 61 forks source link

Ensure that SAML1 and SAML2 names are not empty in attribute's metadata at all times #2417

Open aliaksander-samuseu opened 7 months ago

aliaksander-samuseu commented 7 months ago

Description

This issue is an enhancement proposal. As was recently discovered while troubleshooting issue 11761, when attribute's metadata's properties "gluuSAML1URI" and "gluuSAML2URI" are not assigned a value, that leads to oxTrust crash during startup, due to its inability to gather all required data to generate configuration files for Shibboleth IDP. The issue has been confirmed for Gluu Server using SQL db for persistence so far.

Steps To Reproduce

  1. Edit some attribute by removing values from its "SAML2 URI" and "SAML1 URI" properties (oxTrust allows to save attribute's properties when these fields are empty)
  2. Create a SAML TR and add this attribute to its "Released" list of attributes
  3. Restart oxTrust's service/pod

Results

oxTrust fails to start, and error like shown below is displayed: attr_issue SQL request like this - select gluuSAML1URI,gluuSAML2URI from gluuAttribute where gluuAttributeName = "birthdate"; returns next output:

+--------------+--------------+
| gluuSAML1URI | gluuSAML2URI |
+--------------+--------------+
| NULL         | NULL         |
+--------------+--------------+

Conclusion

As these properties seem to be mandatory, it would make sense to add a safeguard to oxTrust's code preventing user from assigning an empty value to them - both from web UI and from oxTrust API. Otherwise it may result in admin locked out of admin console due to some arbitrary configuration edit (that's what happened in the original ticket).

aliaksander-samuseu commented 7 months ago

A quick research on this error's origin I did when we were troubleshooting the crash in the call: attr_issue2 Based on a cursory reading and some observations while testing it, the code compiles a list of all attributes currently referenced from all active TRs, and then tries to build a mapping between their "Gluu names" and SAML names, to put the latter inside IDP config files it's about to generate. When it can't retrieve SAML names for some attributes, null is returned, and the startup routine breaks.

So it seems we need to either make it fail gracefully in such case, silently excluding attribute for which SAML names are not defined (and perhaps change that log message to state the reason of it a bit more clearly) - or acknowledge these are mandatory properties, and thus oxTrust must not persist any edits that result in no values set for them.