Skip to content

SAML2 frontend+backend: support reloading metadata #370

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

Closed

Conversation

vladimir-mencl-eresearch
Copy link
Contributor

@vladimir-mencl-eresearch vladimir-mencl-eresearch commented Jun 11, 2021

Using the reload_metadata method added into pysaml2 in IdentityPython/pysaml2#809, support reloading metadata when triggered via an externally exposed URL (as /<module_name>/reload-metadata)

This is off by default (URL not exposed) and needs to be explicitly enabled by setting the newly introduced config option enable_metadata_reload for the SAML modules to true (or yes).

The loaded config is already preserved in the modules, so can be easily used to provide a reference copy of the metadata configuration to the reload_metadata method.

This is implemented separately for the SAML2 Backend and SAML2 Frontend (applying to all three SAML2 Frontend classes).

This will complete the missing functionality identified in IdentityPython/pysaml2#808

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • [Not yet] Have you written new tests for your changes?
  • [N/A] Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

Using the reload_metadata method added into pysaml2 in IdentityPython/pysaml2#809,
support reloading metadata when triggered via an externally exposed URL
(as `/<module_name>/reload-metadata`)

This is off by default (URL not exposed) and needs to be explicitly enabled
by setting the newly introduced config option `enable_metadata_reload`
for the SAML modules to `true` (or `yes`).

The loaded config is already preserved in the modules, so can be easily used
to provide a reference copy of the metadata configuration to the `reload_metadata` method.

This is implemented separately for the SAML2 Backend and SAML2 Frontend
(applying to all three SAML2 Frontend classes).

This will complete the missing functionality identified in IdentityPython/pysaml2#808
Copy link
Member

@c00kiemon5ter c00kiemon5ter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made some suggestions on the structure. Let me know what you think.

@vladimir-mencl-eresearch
Copy link
Contributor Author

Hi @c00kiemon5ter ,

Thanks also for this review.

I agree with the motivation for a generic method - I myself wasn't so comfortable with duplicating the code between the two copies of _reload_metadata in SAMLBackend and SAMLFrontend.

The two methods have very few differences - but besides what they use as config, it's also what object they invoke reload_metadata on - self.sp vs. self.idp.

So the proposed change would not work for SP metadata updates - it would be calling self.idp.reload_metadata, and self.idp would not exist for an SP.

We could also go one step further and add an additional target parameter to the generic form (being self.idp or self.sp), or stick with the original form ...

I initially decided to do two separate methods, after considering what shape a generic one would have to take, but happy to implement that if you'd think that would work out better.

Cheers,
Vlad

@c00kiemon5ter
Copy link
Member

rebased and merged by b78ab3e and 206d55d

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.

2 participants