Open theseal opened 11 months ago
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:
length(entity_descriptors) > 1
would be more likely to cause issuesNote 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.
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.
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: