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

Ensure DigiD/eHerkenning metadata doesn't need to be manually uploaded and is automatically retrieved #45

Closed
alextreme opened this issue Oct 6, 2023 · 5 comments · Fixed by #46
Assignees

Comments

@alextreme
Copy link
Member

Currently it is necessary to manually upload the Metadata identity provider (idp_metadata_file) and specify the Identity provider service ID (idp_service_entity_id).

image

This is weird because you download the metadata from the same URL as filled in for idp_service_entity_id and upload it as the idp_metadata_file.

In my eyes it would make more sense to allow the idp_metadata_file and idp_service_entity_id to be empty and provide a Identity provider metadata URL field that when pointing to the metadata auto-populates the two existing fields. Or remove the idp_metadata_file and idp_service_entity_id fields altogether, as the URL to the metadata is all you need.

If doing so, note that this metadata changes. New DigiD certificates for instance are added to the metadata when the old ones expire, which is the case next monday requiring all admins to manually refresh the DigiD metadata. Laurens had to do the same this wednesday for all OF testenvironments individually (I'm glad I only have a few OIP environments to do the same).

A management command would be useful that automatically updates the stored metadata, or the metadata could be retried on-the-fly when accessing DigiD and cached.

To be discussed with @sergei-maertens because I'm not sure why the current approach was chosen, after which @Viicos can take a look to improve this.

@alextreme
Copy link
Member Author

Info vanuit Logius:

https://www.logius.nl/actueel/digid-vervanging-saml-signing-certificaat

Om ervoor te zorgen dat uw webdienst na de vervanging blijft werken, is het belangrijk dat u de nieuwe SAML-metadata van DigiD inleest in uw SAML-applicatie. Bij veel SAML-applicaties gebeurt dit automatisch zodra wij in het onderhoudsmoment de SAML-metadata hebben vervangen. Bij sommige SAML-applicaties is het echter nodig dat u de SAML-metadata handmatig inleest via de URL die we hiervoor beschikbaar hebben. Dit dient u dan te doen direct ná het onderhoudsmoment.

@sergei-maertens
Copy link
Member

To be discussed with @sergei-maertens because I'm not sure why the current approach was chosen

I've no idea - I just cleaned up the code and moved existing settings-based configuration to the admin interface. From what I understood though, the IDP ID is not guaranteed to be a URL, for certain brokers these can be a urn: value, but I really know too little about all these details to be able to accurately judge this.

I do definitely want to explore how this can be automated, as that's what Logius is hinting at.

@SilviaAmAm
Copy link
Contributor

SilviaAmAm commented Oct 6, 2023

Related: #44

Indeed, the IdP entity Id can be a string of shape urn:etoegang:HM:00000001231231230000:entities:9123 for some eHerkenning brokers. However, this value should be in the metadata.

So if the IdP provides a link where we can retrieve the metadata, we should be able to automatically extract the entityID from the metadata 🤔

@sergei-maertens
Copy link
Member

Additional context after talking to Alex - metadata can contain multiple IdP entity IDs, so that's why it needs to be specified in the config in the event that there are multiple.

It would be best to extract the (first) entity ID and store it in the config, and if there are multiple, then the administrator can change the value. Making a dropdown of the available values instead of a text field would also be nice I suppose!

@vaszig vaszig added the wip Work in progress label Oct 11, 2023
vaszig added a commit that referenced this issue Oct 13, 2023
Refactored base configuration model in order to provide a url and do all
the fetching and parsing based on this. The urls are cached for a day
and the xml file is updated via a command.
vaszig added a commit that referenced this issue Oct 16, 2023
Refactored base configuration model in order to provide a url and do all
the fetching and parsing based on this. The urls are cached for a day
and the xml file is updated via a command.
@vaszig vaszig removed the wip Work in progress label Oct 17, 2023
@vaszig
Copy link
Contributor

vaszig commented Oct 17, 2023

After the last discussion with @alextreme we concluded that EntityID will be one each time so the implementation (#46) will be according to this.

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 a pull request may close this issue.

4 participants