IdentityPython / SATOSA

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

new: add microservice to expose info about IdP from SAML metadata as attributes #448

Open vladimir-mencl-eresearch opened 10 months ago

vladimir-mencl-eresearch commented 10 months ago

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:

vladimir-mencl-eresearch commented 8 months ago

@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.

c00kiemon5ter commented 8 months ago

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 (<attr>#{language}).

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.

vladimir-mencl-eresearch commented 8 months ago

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 metainfo plugin, it is to a degree similar to what this PR does - a ResponseMicroService that extracts useful bits out of the metadata and stores it somewhere. It differs in where it gets stored - my approach puts it into attributes, metainfo puts it into a separate part of the internal state. (And it so far misses RegistrationAuthority which is critical in my use case).

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 metainfo (or reusing it directly) to store the metadata info in internal state, and separate service for copying the internal state into attributes?

Please let me know.

Cheers, Vlad

c00kiemon5ter commented 8 months ago

And it so far misses RegistrationAuthority which is critical in my use case

it should be part of the registration info

The part I don't get so far is how would get used further on in the attribute state.

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. For the case you describe, the possible communication channels for this information is

What are your thoughts - should I be heading this way? Separate service like metainfo (or reusing it directly) to store the metadata info in internal state, and separate service for copying the internal state into attributes?

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.

vladimir-mencl-eresearch commented 8 months ago

Hi @c00kiemon5ter ,

Thanks for the patient discussion!

This all started with one of our members asking the question:

So, if you connect our service (via SATOSA OIDC/SAML proxy) into eduGAIN, how do we tell whether the user is from our own federation or remote?

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 metadataAttributePrefix.

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 id-token / access-token / userinfo endpoint, in what form did you mean that? Other part of the structure then the list of attributes? Anything specific?

Thanks again for the patience in this discussion.

Cheers, Vlad

c00kiemon5ter commented 8 months ago

When you suggest to expose this information in id-token / access-token / userinfo endpoint, in what form did you mean that? Other part of the structure then the list of attributes? Anything specific?

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. Which of those mechanism you are going to use depends on the semantics given by the registration-authority information:

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. The metadata itself can be used for other purposes too. Thus, it makes sense to have a plugin that makes them available for processing, and have a separate plugin to map them in the attributes struct, and have a separate plugin to evaluate whether certain attributes should be dropped because of some signal in the metadata, etc.

vladimir-mencl-eresearch commented 8 months ago

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 metainfo plugin and the swamid-satosa code/repo is?

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, Vlad