-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new: add microservice to expose info about IdP from SAML metadata as attributes #448
base: master
Are you sure you want to change the base?
new: add microservice to expose info about IdP from SAML metadata as attributes #448
Conversation
…attributes Primary goal was to expose registration_authority for services connected to eduGAIN. Generalised to expose all possibly useful info available from MDStore. Notable decisions: * while registration_authority is included in registration_info data structure, exposing it directly should make it easier for applications to consume and process the value. * to expose scopes, it was necessary to revert the regex_compile step done when regexp="true" (as Pattern objects are not seriazable) * tuple objects are not serializable and had to be converted to a list * mod_auth_openidc does not accept lists of complex types as claim value, assuming this might also apply to other RP implementations, wrap the listt a dict to match other attributes.
@c00kiemon5ter , just wondering if I could get your thoughts on this - at least whether you see this extension as useful in principle (and find the approach suitable). If so, I'd add tests and finalise the PR. |
Thank you for being patient @vladimir-mencl-eresearch I don't think we should follow this approach. Mixing attributes with metadata is confusing and can lead to false assumptions and issues. Additionally, the information is about the metadata of the issuer; there is no place for the metadata of the service, which is also important for certain decisions. Instead, I think that we should follow what was done with the metainfo plugin here. The idea is that a new slot is set on the InternalData instance which holds the metadata of the requester and of the issuer that are involved in this flow separately (with some conventions for OIDC entities). In addition to the above, I liked the notation that was set on #438 to signify the language of a value, without using a hierarchy ( The metainfo plugin needs some further work and some considerations to be taken into account. For example
Ideally, I'd like to push this responsibility to the backends and frontends. Those modules would prepare the metadata and pass it on to satosa-core to be exposed to the plugins. That would mean that we need an API to call into these modules and an internal format for the metadata structure. |
Hi @c00kiemon5ter , Thanks for the response and review - very much appreciated. This is also why I sent the PR in at an early stage to get your opinion, to allow me to shape it properly. I'm trying to get what approach you'd prefer instead. Looking at the The part I don't get so far is how would get used further on in the attribute state. What I'm aiming to do is to expose the info (in particular RegistrationAuthority) to OIDC clients - so that when the SATOSA SAML SP is connected to eduGAIN, they can tell which federation the user came from (or can at least distinguish between local federation and an eduGAIN user). There could be a separate microservice that would map the metadata state to attributes ... but that would be getting rather close to my original PR. This approach would have the advantage that it gets only mapped to attributes when explicitly requested so - but the same could be said about my original PR. Well, the split approach would allow to use the metadata info without polluting the attributes. What are your thoughts - should I be heading this way? Separate service like Please let me know. Cheers, |
it should be part of the registration info
either you create a new micro-service an map the metadata values that you want to attributes/claims, or this is offloaded to the frontend that will pick up this information from the internal data and use it as needed.
Yes, I think they are different concerns and should be separate. Further down the road, if the metadata is provided by the backends and frontends and is part of the InternalData object before the processing of the micro-services/plugins, then the micro-service that maps metadata to attributes/claims should keep working unaffected. |
Hi @c00kiemon5ter , Thanks for the patient discussion! This all started with one of our members asking the question:
Which very much makes sense to me - and I've been looking for ways to expose this information (registrationAuthority) to them. (And sorry, I missed registrationInfo was covered by the metainfo plugin - can see it now). Given the OIDC client/RP is a Keycloak instance and the code evaluating the user information will be further down, I thought of additional attributes as the best way forward. I took inspiration from Shibboleth SP's MetadataAttributeExtractor which does exactly this: extracts values from metadata of the IdP issuing the SAML response and adds them to the set of attributes received. The attribute names can all be assigned into a separate "namespace" by prefixing the attribute name with a value set in One option for tweaking this PR would be to prefix all attribute names with a prefix indicating these come from the metadata (changing it in the example config). When you suggest to expose this information in Thanks again for the patience in this discussion. Cheers, |
As a claim in the token, or a claim in the userinfo response (or, a claim in the introspection response). There is no other structure there than the claims.
Try to understand if the knowledge of this property gives the user access to more resources (thus, it is authorization information / access-token and introspection), or it is an unstable property of the current flow (thus, it is session information / id-token), or if it is part of the user identity (userinfo). To be clear, I don't think that what you are trying to do is wrong. What I do not like is mixing the metadata extraction process, with the metadata-to-attribute mapping process. Especially if the metadata extraction process is going to move outside of the micro-services. |
Thanks @c00kiemon5ter . You are overseeing the overall architecture of SATOSA - so thanks for keeping an eye on that. I agree the approach with separate services for extracting metadata info and for mapping it to attributes is a better choice. To get this to usable state, I understand I'd have to write the second half as new code. As for the first half, do you know what the position/state of the Should I import the code from there (crediting original authors ... so you :-) - or should it stay in that repo? Is that repo actively maintained / would accept PRs and publish releases ? I expect it would look odd to have the part mapping the internal state metadata info to attributes in SATOSA but the metainfo plgun populating this state only in the separate swamid-SATOSA repo... Thanks a lot for this discussion - I hope we can converge to a workable plan. Cheers, |
Primary goal was to expose registration_authority for services connected to eduGAIN.
Generalised to expose all possibly useful info available from MDStore.
Notable decisions:
Please let me know what you think @c00kiemon5ter - happy to add tests once I get the overall OK.
All Submissions: