Improvement suggestions for SAML metadata schema
Description
Environment
Activity
Scott Cantor March 4, 2021 at 8:36 PM
After Slack discussion, pulled trigger to revamp schema. Moved all the singletons into XML Attributes. Changed non-URI valued lists to XML Attributes with lists. Renamed everything but the credential elements to match OAuth/OIDC claim names so that no translating is required.
Scott Cantor March 4, 2021 at 1:36 PM
They aren't Spring files so there isn't any way to do property replacement. This is one case where that might not be a totally worthless feature but I'm not sure how we'd do it even if we wanted to.
Rod Widdowson March 4, 2021 at 1:30 PM
Forgive me that I have not been following closely but
The schema is using local string, anyURI, and listOfString types to impose stricter value rules since we control the schema fully.
bear in mind that property replacement has a habit of screwing you....
url="%{oidc.url}"
may not parse as a URI, but it might represent one.
Or I may have the wrong end of that stick
Scott Cantor March 3, 2021 at 8:58 PM
I committed a round of small changes (and one major fix) while I go through everything.
The schema is using local string, anyURI, and listOfString types to impose stricter value rules since we control the schema fully.
I also extended a small number of fields to allow XML lists so that a single element in some cases lets you capture the list of values more elegantly. The wiki docs carry some notes about which fields allow that.
The major bugs were around the JWK elements, because the names in the document and the code didn't line up with the XMLObject config file that defines all the implementations. I cleaned that up (I think).
The big bug was that the docs and example files suggested that the JwksUri element was meant to be inside KeyInfo, not in the "root" extension element, but the code was processing it strictly from the root element and not KeyInfo.
I took a stab at fixing that by walking the Credentials list looking for the appropriate "resolved" type. I also adjusted the code to "favor" the URI over the inline data case and only set one, which is what the specs actually seem to say. Of course we believe using a URI is extremely insecure but that insecurity is why it works for key rotation and fixing OIDC insecurity is not in scope here.
Scott Cantor March 3, 2021 at 6:16 PMEdited
Defined full URI constants for the OIDC sub formats for NameIDFormat use.
I'd suggest we make some improvements to the metadata schema, in particular considering moving to XML lists of strings in some of the elements that map pretty naturally to JSON arrays, such as the response and grant types. SAML certainly makes use of lists at times so it's not a foreign idea.
We probably should also tighten up the types so that we can preclude empty elements to begin with, unlike SAML's mistake.
I can take a bit of time to review things and make some specific suggestions now that I have a working sample.