SATOSA icon indicating copy to clipboard operation
SATOSA copied to clipboard

Handle multiple back/front-ends

Open theseal opened this issue 1 year ago • 3 comments

Without this fix only the last back/front-end will be written to file if split is not involved.

Add new method create_entities_descriptor as a counterpart to create_signed_entity_descriptor to also apply valid option to EntititesDescriptor but avoiding signing.

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?
  • [ ] Have you written new tests for your changes?
  • [ ] Does your submission pass tests?
  • [ ] This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

theseal avatar Dec 19 '23 15:12 theseal

Hi,

Looking at this PR, I understand this would introduce a breaking change: even for deployments with just a entity entity descriptor, the EntityDescriptor would now be wrapped in an EntitiesDescriptor. This might take many deployments by surprise - break the unwritten assumption that the file would contain just a single EntityDescriptor element.

Though, I'm not sure what the best way forward would be:

  • doing this conditional on number of length(entity_descriptors) > 1 would be more likely to cause issues
  • maybe a new option to wrap multiple descriptors into an EntitiesDescriptor?
  • or accept this PR and flag it as a breaking change

Note that I'm not a maintainer, so these are just my thoughts, not a maintainer review.

Hope its helpful.

Cheers, Vlad

You are right and thought about it when I started to write the change but apparently forgot about it.

We have to wonder what the intent was when the code was written from the begging I guess. Stacking multiple entityDescriptor's without embracing in an entitiesDescriptor I think is incorrect.

Not sure how common it is to have multiple front/back-ends (we have at-least) so to mitigate the break I could add your suggested check for the amount of entities and only wrap multiple entities in an entitiesDescriptor.

theseal avatar Dec 20 '23 07:12 theseal

I have a use case where I can have an arbitrary number of SAML backends (each service bridged has a standalone SAML identity), but I use the --split-backend to get each EntityDescriptor in a separate metadata file.