IdentityPython / SATOSA

Proxy translating between different authentication protocols (SAML2, OpenID Connect and OAuth2)
https://idpy.org
Apache License 2.0
197 stars 121 forks source link

`extra_scopes` are always of type `list` #457

Open marcvs opened 6 months ago

marcvs commented 6 months ago

Expected Behavior

extra_scopes MUST be definable to be of ANY type!

I am checking the attributes sent from various OIDC-OPs, and all of the satosa instances (eduTeams, didmos) show this problem. Most pressing is the fact that "string" type attributes are sent as a "list of strings", which is against the spec, of e.g. org_domain, or schac_home_organization. Clients my break.

I theroy, bool type extra_scopes are possible, also "emptye list" too.

Steps to Reproduce

See here: https://cvs.data.kit.edu/~naco/

ar-filho commented 5 months ago

I'm using https://github.com/UniversitaDellaCalabria/SATOSA-oidcop, and I opened an Issue there. However, it was recommended that this modification occurs in SATOSA and not in the plugin.

For now we are doing this in the plugin: https://github.com/UniversitaDellaCalabria/SATOSA-oidcop/issues/50

I was analyzing how to suggest this update to SATOSA, and I think it could be an alternative to put an identifier in internal_attributes.yaml informing that that attribute is unique. This way, I believe it would resolve it. Thinking about OIDC, some attributes such as address will still be sent as lists.

c00kiemon5ter commented 4 months ago

You are right that this is happening atm. I agree with @ar-filho that this should be part of the information in the internal attributes, to specify rules for the multiplicity, the type, and even comparison options for the attributes.

Doing this with a separate structure can work. Otherwise we need to extend the existing structure, but it breaks existing configurations. I think however, that we can allow to parse both the old and new format and push everyone to use the new format.

The benefit of extending the existing structure is that we can ensure that the rules are in place for all attributes. With separate structure we need to find the corresponding entries or fallback to defaults. Inversely, the new structures can have more elements that the ones in the existing struct; what would that mean? It could be that we are introducing confusion and making the config more error prone by separating the structs.

I am thinking about it. Any thoughts are welcome.