-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Add 8.19 transport version for IdP Extension Attr #129233
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
Add 8.19 transport version for IdP Extension Attr #129233
Conversation
This adds a new patch level TransportVersion in preparation for backporting elastic#128805
Pinging @elastic/es-security (Team:Security) |
@@ -121,6 +147,9 @@ private SamlServiceProviderDocument createFullDocument() throws GeneralSecurityE | |||
doc1.attributeNames.setEmail("urn:" + randomAlphaOfLengthBetween(4, 8) + "." + randomAlphaOfLengthBetween(4, 8)); | |||
doc1.attributeNames.setName("urn:" + randomAlphaOfLengthBetween(4, 8) + "." + randomAlphaOfLengthBetween(4, 8)); | |||
doc1.attributeNames.setRoles("urn:" + randomAlphaOfLengthBetween(4, 8) + "." + randomAlphaOfLengthBetween(4, 8)); | |||
doc1.attributeNames.setExtensions( | |||
randomList(0, 3, () -> "urn:" + randomAlphaOfLengthBetween(4, 8) + "." + randomAlphaOfLengthBetween(4, 8)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testStreamRoundTripWithAllFields
now fails randomly in assertToXContentEquivalent
. The problem seem to be with the ordering of extensions
items:
java.lang.AssertionError: Error when comparing xContent.
acs: same [https://cEtrLOYX.LakSo/saml/acs]
attributes:
email: same [urn:hAVRt.inLOJA]
extensions:
0: expected String [urn:pDeptYdD.rehxgHP] but was String [urn:BeZmekTl.QhNbhUns]
1: expected String [urn:BeZmekTl.QhNbhUns] but was String [urn:pDeptYdD.rehxgHP]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll just make it a single element then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This extends the change from #128176 to validate the "custom attributes" on a per Service Provider basis. Each Service Provider (whether registered or wildcard based) has a field "attributes.extensions" which is a list of attribute names that may be provided by the caller of "/_idp/saml/init". Service Providers that have not be configured with extension attributes will reject any custom attributes in SAML init. This necessitates a new field in the service provider index (but only if the new `extensions` attribute is set). The template has been updated, but there is no data migration because the `saml-service-provider` index does not exist in any of the environments into which we wish to deploy this change. Backport of: #128805, #129233
This adds a new patch level TransportVersion in preparation for backporting #128805