Skip to content
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

Feature/19 local dynamic metadata provider #44

Open
wants to merge 6 commits into
base: maint-1
Choose a base branch
from

Conversation

brzler
Copy link

@brzler brzler commented Mar 31, 2020

Adds the OIDC-equivalent of Shibboleth's LocalDynamicMetadataProvider (https://wiki.shibboleth.net/confluence/display/SP3/MetadataProvider).

Uses Java NIO2's WatchService to handle Metadata file changes.

@brzler
Copy link
Author

brzler commented Mar 31, 2020

for feature request #19

@Sheldor5
Copy link

exactly what was missing, hope this gets merged

@tbrandstetter
Copy link

@scantor are you going to merge this feature to the Shibboleth OIDC version 3.x? It would be a great benefit like the LocalDynamicMetadataProvider for SAML2.

@scantor
Copy link
Collaborator

scantor commented Apr 2, 2021

Not sure you're asking me specifically, but my answer would be just use SAML metadata and the existing LocalDynamic provider.

@tbrandstetter
Copy link

I'm looking for a solution for OIDC metadata (json files). For SAML the existing LocalDynamic provider works without a problem. Reading the docs of Shibboleth for the new OIDC OP version 3.0.0 there is only the filesystem metadata provider (with a static json) and the storage metadata provider for dynamic registrations available.

This pull request is closing the gap between the SAML dynamic metadata and OIDC. Thus, I'm asking for implementation in Shibboleth if possible. Sorry, if this is the wrong place - but the pull request is here and ready. Furthermore, it seems that this repository is abandoned and ongoing development is taking place in Shibboleth now.

@scantor
Copy link
Collaborator

scantor commented Apr 2, 2021

Henri is still the lead on that part of the code, but this isn't where we're tracking issues anymore for the code now.

As for the request, we support and greatly favor SAML metadata for OIDC, not JSON. The IdP bases most of its processing on that format and we cannot do the same things with the limited capabilities of this format. The 3.0 plugin does not solely support the JSON format, it supports SAML metadata, which is documented.

@tbrandstetter
Copy link

Thanks for your support. Seems I misunderstood the documentation. It's clear for me now.

@scantor
Copy link
Collaborator

scantor commented Apr 2, 2021

Docs are a work in progress (to make a serious understatement), I'll take a look and see if it needs more context.

My concern with this kind of addition is that it's adding a big pile of duplicate code overlapping what we have now, so if we did it at all, I can bet it would have to be redone and based on the actual LocalDynamic classes (maybe this patch is, but I don't think so).

Ultimately really up to @hjmikkon whether we want to think about it but my hope is in practice it will become less interesting when people better grasp how powerful metadata-driven configuration is in the IdP. That also opens up the ability for Unicon's GUI to be used to manage OIDC metadata and settings.

@tbrandstetter
Copy link

https://wiki.shibboleth.net/confluence/display/SC/OAuthRPMetadataProfile?src=contextnavpagetreemode was the missing link :-)

I'm sure it is working this way and because of that I share your opinion about avoiding duplicate code. It makes no sense to maintain two places of metadata repositories for one identity provider.

Again, thanks for the fast and professional support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants