Skip to content

Commit b24fff4

Browse files
lloydmetatvernum
lloydmeta
and
tvernum
authored
[8.19][Backport] Implement SAML custom attributes support for Identity Provider (#128796)
* Implement SAML custom attributes support for Identity Provider (#128176) * Implement SAML custom attributes support for Identity Provider This commit adds support for custom attributes in SAML single sign-on requests in the Elasticsearch X-Pack Identity Provider plugin. This feature allows passage of custom key-value attributes in SAML requests and responses. Key components: - Added SamlInitiateSingleSignOnAttributes class for holding attributes - Added validation for null and empty attribute keys - Updated request and response objects to handle attributes - Modified authentication flow to process attributes - Added test coverage to validate attributes functionality The implementation follows Elasticsearch patterns with robust validation and serialization mechanisms, while maintaining backward compatibility. * Add test for SAML custom attributes in authentication response This commit adds a comprehensive test that verifies SAML custom attributes are correctly handled in the authentication response builder. The test ensures: 1. Custom attributes with single and multiple values are properly included 2. The response with custom attributes is still correctly signed 3. The XML schema validation still passes with custom attributes 4. We can locate and verify individual attribute values in the response This provides critical test coverage for the SAML custom attributes feature implementation. * Add backward compatibility overload for SuccessfulAuthenticationResponseMessageBuilder.build This commit adds an overloaded build method that accepts only two parameters (user and authenticationState) and forwards the call to the three-parameter version with null for the customAttributes parameter. This maintains backward compatibility with existing code that doesn't use custom attributes. This fixes a compilation error in ServerlessSsoIT.java which was still using the two-parameter method signature. Signed-off-by: lloydmeta <[email protected]> * Add validation for duplicate SAML attribute keys This commit enhances the SAML attributes implementation by adding validation for duplicate attribute keys. When the same attribute key appears multiple times in a request, the validation will now fail with a clear error message. Signed-off-by: lloydmeta <[email protected]> * Refactor SAML attributes validation to follow standard patterns This commit improves the SAML attributes validation by: 1. Adding a dedicated validate() method to SamlInitiateSingleSignOnAttributes that centralizes validation logic in one place 2. Moving validation from constructor to dedicated method for better error reporting 3. Checking both for null/empty keys and duplicate keys in the validate() method 4. Updating SamlInitiateSingleSignOnRequest to use the new validation method 5. Adding comprehensive tests for the new validation approach These changes follow standard Elasticsearch validation patterns, making the code more maintainable and consistent with the rest of the codebase. * Update docs/changelog/128176.yaml * Improve SAML response validation in identity provider tests Enhanced the testCustomAttributesInIdpInitiatedSso test to properly validate both SAML response structure and custom attributes using DOM parsing and XPath. Key improvements: - Validate SAML Response/Assertion elements exist - Precisely validate custom attributes (department, region) and their values - Use namespace-aware XML parsing for resilience to format changes Signed-off-by: lloydmeta <[email protected]> * Simplify SAML attributes representation using JSON object/Map structure Also, replace internal Attribute class list with a simpler Map<String, List<String>> structure This change: - Removes the redundant Attribute class and replaces it with a direct Map implementation for storing attribute key-value pairs - Eliminates the duplicate "attributes" nesting in the JSON structure - Simplifies attribute validation without needing duplicate key checking - Updates all related tests and integration points to work with the new structure Before: ```js { // others "attributes": { "attributes": [ { "key": "department", "values": ["engineering", "product"] } ] } } After: ```js { // other "attributes": { "department": ["engineering", "product"] } } ``` (Verified by spitting out JSON entity in IdentityProviderAuthenticationIT.generateSamlResponseWithAttributes ... saw `{"entity_id":"ec:123456:abcdefg","acs":"https://sp1.test.es.elasticsearch.org/saml/acs","attributes":{"department":["engineering","product"],"region":["APJ"]}}`) Signed-off-by: lloydmeta <[email protected]> * * Fix up toString dangling quote. Signed-off-by: lloydmeta <[email protected]> * * Remove attributes from Response object. Signed-off-by: lloydmeta <[email protected]> * * Remove friendly name. * Make attributes map final in SamlInitiateSingleSignOnAttributes Signed-off-by: lloydmeta <[email protected]> * * Cleanup serdes by using existing utils in the ES codebase Signed-off-by: lloydmeta <[email protected]> * Touchup comment Signed-off-by: lloydmeta <[email protected]> * Update x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnRequestTests.java Co-authored-by: Tim Vernum <[email protected]> * Add transport-version checks --------- Signed-off-by: lloydmeta <[email protected]> Co-authored-by: Tim Vernum <[email protected]> * * TV fixups Signed-off-by: lloydmeta <[email protected]> --------- Signed-off-by: lloydmeta <[email protected]> Co-authored-by: Tim Vernum <[email protected]>
1 parent 2a2adcc commit b24fff4

File tree

11 files changed

+694
-31
lines changed

11 files changed

+694
-31
lines changed

‎docs/changelog/128176.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 128176
2+
summary: Implement SAML custom attributes support for Identity Provider
3+
area: Authentication
4+
type: enhancement
5+
issues: []

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ static TransportVersion def(int id) {
229229
public static final TransportVersion ML_INFERENCE_SAGEMAKER_CHAT_COMPLETION_8_19 = def(8_841_0_37);
230230
public static final TransportVersion ML_INFERENCE_VERTEXAI_CHATCOMPLETION_ADDED_8_19 = def(8_841_0_38);
231231
public static final TransportVersion INFERENCE_CUSTOM_SERVICE_ADDED_8_19 = def(8_841_0_39);
232-
232+
public static final TransportVersion IDP_CUSTOM_SAML_ATTRIBUTES_ADDED_8_19 = def(8_841_0_40);
233233
/*
234234
* STOP! READ THIS FIRST! No, really,
235235
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _

‎x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java‎

Lines changed: 119 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,38 @@
1818
import org.elasticsearch.common.util.concurrent.ThreadContext;
1919
import org.elasticsearch.core.Nullable;
2020
import org.elasticsearch.xcontent.ObjectPath;
21+
import org.elasticsearch.xcontent.XContentBuilder;
2122
import org.elasticsearch.xcontent.json.JsonXContent;
2223
import org.elasticsearch.xpack.core.security.action.saml.SamlPrepareAuthenticationResponse;
2324
import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProviderIndex;
2425
import org.junit.Before;
26+
import org.w3c.dom.Document;
27+
import org.w3c.dom.Element;
28+
import org.w3c.dom.NodeList;
29+
import org.xml.sax.InputSource;
2530

2631
import java.io.IOException;
32+
import java.io.StringReader;
2733
import java.nio.charset.StandardCharsets;
34+
import java.util.ArrayList;
2835
import java.util.Base64;
2936
import java.util.List;
3037
import java.util.Map;
3138
import java.util.Set;
3239

40+
import javax.xml.parsers.DocumentBuilder;
41+
import javax.xml.parsers.DocumentBuilderFactory;
42+
import javax.xml.xpath.XPath;
43+
import javax.xml.xpath.XPathConstants;
44+
import javax.xml.xpath.XPathFactory;
45+
3346
import static org.hamcrest.Matchers.containsInAnyOrder;
3447
import static org.hamcrest.Matchers.containsString;
3548
import static org.hamcrest.Matchers.equalTo;
3649
import static org.hamcrest.Matchers.hasSize;
3750
import static org.hamcrest.Matchers.instanceOf;
51+
import static org.hamcrest.Matchers.is;
52+
import static org.hamcrest.Matchers.notNullValue;
3853

3954
public class IdentityProviderAuthenticationIT extends IdpRestTestCase {
4055

@@ -74,6 +89,81 @@ public void testRegistrationAndIdpInitiatedSso() throws Exception {
7489
authenticateWithSamlResponse(samlResponse, null);
7590
}
7691

92+
public void testCustomAttributesInIdpInitiatedSso() throws Exception {
93+
final Map<String, Object> request = Map.ofEntries(
94+
Map.entry("name", "Test SP With Custom Attributes"),
95+
Map.entry("acs", SP_ACS),
96+
Map.entry("privileges", Map.ofEntries(Map.entry("resource", SP_ENTITY_ID), Map.entry("roles", List.of("sso:(\\w+)")))),
97+
Map.entry(
98+
"attributes",
99+
Map.ofEntries(
100+
Map.entry("principal", "https://idp.test.es.elasticsearch.org/attribute/principal"),
101+
Map.entry("name", "https://idp.test.es.elasticsearch.org/attribute/name"),
102+
Map.entry("email", "https://idp.test.es.elasticsearch.org/attribute/email"),
103+
Map.entry("roles", "https://idp.test.es.elasticsearch.org/attribute/roles")
104+
)
105+
)
106+
);
107+
final SamlServiceProviderIndex.DocumentVersion docVersion = createServiceProvider(SP_ENTITY_ID, request);
108+
checkIndexDoc(docVersion);
109+
ensureGreen(SamlServiceProviderIndex.INDEX_NAME);
110+
111+
// Create custom attributes
112+
Map<String, List<String>> attributesMap = Map.of("department", List.of("engineering", "product"), "region", List.of("APJ"));
113+
114+
// Generate SAML response with custom attributes
115+
final String samlResponse = generateSamlResponseWithAttributes(SP_ENTITY_ID, SP_ACS, null, attributesMap);
116+
117+
// Parse XML directly from samlResponse (it's not base64 encoded at this point)
118+
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
119+
factory.setNamespaceAware(true); // Required for XPath
120+
DocumentBuilder builder = factory.newDocumentBuilder();
121+
Document document = builder.parse(new InputSource(new StringReader(samlResponse)));
122+
123+
// Create XPath evaluator
124+
XPathFactory xPathFactory = XPathFactory.newInstance();
125+
XPath xpath = xPathFactory.newXPath();
126+
127+
// Validate SAML Response structure
128+
Element responseElement = (Element) xpath.evaluate("//*[local-name()='Response']", document, XPathConstants.NODE);
129+
assertThat("SAML Response element should exist", responseElement, notNullValue());
130+
131+
Element assertionElement = (Element) xpath.evaluate("//*[local-name()='Assertion']", document, XPathConstants.NODE);
132+
assertThat("SAML Assertion element should exist", assertionElement, notNullValue());
133+
134+
// Validate department attribute
135+
NodeList departmentAttributes = (NodeList) xpath.evaluate(
136+
"//*[local-name()='Attribute' and @Name='department']/*[local-name()='AttributeValue']",
137+
document,
138+
XPathConstants.NODESET
139+
);
140+
141+
assertThat("Should have two values for department attribute", departmentAttributes.getLength(), is(2));
142+
143+
// Verify department values
144+
List<String> departmentValues = new ArrayList<>();
145+
for (int i = 0; i < departmentAttributes.getLength(); i++) {
146+
departmentValues.add(departmentAttributes.item(i).getTextContent());
147+
}
148+
assertThat(
149+
"Department attribute should contain 'engineering' and 'product'",
150+
departmentValues,
151+
containsInAnyOrder("engineering", "product")
152+
);
153+
154+
// Validate region attribute
155+
NodeList regionAttributes = (NodeList) xpath.evaluate(
156+
"//*[local-name()='Attribute' and @Name='region']/*[local-name()='AttributeValue']",
157+
document,
158+
XPathConstants.NODESET
159+
);
160+
161+
assertThat("Should have one value for region attribute", regionAttributes.getLength(), is(1));
162+
assertThat("Region attribute should contain 'APJ'", regionAttributes.item(0).getTextContent(), equalTo("APJ"));
163+
164+
authenticateWithSamlResponse(samlResponse, null);
165+
}
166+
77167
public void testRegistrationAndSpInitiatedSso() throws Exception {
78168
final Map<String, Object> request = Map.ofEntries(
79169
Map.entry("name", "Test SP"),
@@ -125,17 +215,37 @@ private SamlPrepareAuthenticationResponse generateSamlAuthnRequest(String realmN
125215
}
126216
}
127217

128-
private String generateSamlResponse(String entityId, String acs, @Nullable Map<String, Object> authnState) throws Exception {
218+
private String generateSamlResponse(String entityId, String acs, @Nullable Map<String, Object> authnState) throws IOException {
219+
return generateSamlResponseWithAttributes(entityId, acs, authnState, null);
220+
}
221+
222+
private String generateSamlResponseWithAttributes(
223+
String entityId,
224+
String acs,
225+
@Nullable Map<String, Object> authnState,
226+
@Nullable Map<String, List<String>> attributes
227+
) throws IOException {
129228
final Request request = new Request("POST", "/_idp/saml/init");
130-
if (authnState != null && authnState.isEmpty() == false) {
131-
request.setJsonEntity(Strings.format("""
132-
{"entity_id":"%s", "acs":"%s","authn_state":%s}
133-
""", entityId, acs, Strings.toString(JsonXContent.contentBuilder().map(authnState))));
134-
} else {
135-
request.setJsonEntity(Strings.format("""
136-
{"entity_id":"%s", "acs":"%s"}
137-
""", entityId, acs));
229+
230+
XContentBuilder builder = JsonXContent.contentBuilder();
231+
builder.startObject();
232+
builder.field("entity_id", entityId);
233+
builder.field("acs", acs);
234+
235+
if (authnState != null) {
236+
builder.field("authn_state");
237+
builder.map(authnState);
238+
}
239+
240+
if (attributes != null) {
241+
builder.field("attributes");
242+
builder.map(attributes);
138243
}
244+
245+
builder.endObject();
246+
String jsonEntity = Strings.toString(builder);
247+
248+
request.setJsonEntity(jsonEntity);
139249
request.setOptions(
140250
RequestOptions.DEFAULT.toBuilder()
141251
.addHeader("es-secondary-authorization", basicAuthHeaderValue("idp_user", new SecureString("idp-password".toCharArray())))

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnRequest.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
*/
77
package org.elasticsearch.xpack.idp.action;
88

9+
import org.elasticsearch.TransportVersions;
910
import org.elasticsearch.action.ActionRequestValidationException;
1011
import org.elasticsearch.action.LegacyActionRequest;
1112
import org.elasticsearch.common.Strings;
1213
import org.elasticsearch.common.io.stream.StreamInput;
1314
import org.elasticsearch.common.io.stream.StreamOutput;
1415
import org.elasticsearch.xpack.idp.saml.support.SamlAuthenticationState;
16+
import org.elasticsearch.xpack.idp.saml.support.SamlInitiateSingleSignOnAttributes;
1517

1618
import java.io.IOException;
1719

@@ -22,12 +24,16 @@ public class SamlInitiateSingleSignOnRequest extends LegacyActionRequest {
2224
private String spEntityId;
2325
private String assertionConsumerService;
2426
private SamlAuthenticationState samlAuthenticationState;
27+
private SamlInitiateSingleSignOnAttributes attributes;
2528

2629
public SamlInitiateSingleSignOnRequest(StreamInput in) throws IOException {
2730
super(in);
2831
spEntityId = in.readString();
2932
assertionConsumerService = in.readString();
3033
samlAuthenticationState = in.readOptionalWriteable(SamlAuthenticationState::new);
34+
if (in.getTransportVersion().onOrAfter(TransportVersions.IDP_CUSTOM_SAML_ATTRIBUTES_ADDED_8_19)) {
35+
attributes = in.readOptionalWriteable(SamlInitiateSingleSignOnAttributes::new);
36+
}
3137
}
3238

3339
public SamlInitiateSingleSignOnRequest() {}
@@ -41,6 +47,17 @@ public ActionRequestValidationException validate() {
4147
if (Strings.isNullOrEmpty(assertionConsumerService)) {
4248
validationException = addValidationError("acs is missing", validationException);
4349
}
50+
51+
// Validate attributes if present
52+
if (attributes != null) {
53+
ActionRequestValidationException attributesValidationException = attributes.validate();
54+
if (attributesValidationException != null) {
55+
for (String error : attributesValidationException.validationErrors()) {
56+
validationException = addValidationError(error, validationException);
57+
}
58+
}
59+
}
60+
4461
return validationException;
4562
}
4663

@@ -68,17 +85,38 @@ public void setSamlAuthenticationState(SamlAuthenticationState samlAuthenticatio
6885
this.samlAuthenticationState = samlAuthenticationState;
6986
}
7087

88+
public SamlInitiateSingleSignOnAttributes getAttributes() {
89+
return attributes;
90+
}
91+
92+
public void setAttributes(SamlInitiateSingleSignOnAttributes attributes) {
93+
this.attributes = attributes;
94+
}
95+
7196
@Override
7297
public void writeTo(StreamOutput out) throws IOException {
7398
super.writeTo(out);
7499
out.writeString(spEntityId);
75100
out.writeString(assertionConsumerService);
76101
out.writeOptionalWriteable(samlAuthenticationState);
102+
if (out.getTransportVersion().onOrAfter(TransportVersions.IDP_CUSTOM_SAML_ATTRIBUTES_ADDED_8_19)) {
103+
out.writeOptionalWriteable(attributes);
104+
}
77105
}
78106

79107
@Override
80108
public String toString() {
81-
return getClass().getSimpleName() + "{spEntityId='" + spEntityId + "', acs='" + assertionConsumerService + "'}";
109+
return getClass().getSimpleName()
110+
+ "{"
111+
+ "spEntityId='"
112+
+ spEntityId
113+
+ "', "
114+
+ "acs='"
115+
+ assertionConsumerService
116+
+ "', "
117+
+ "attributes='"
118+
+ attributes
119+
+ "'}";
82120
}
83121

84122
}

‎x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/TransportSamlInitiateSingleSignOnAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ protected void doExecute(
139139
identityProvider
140140
);
141141
try {
142-
final Response response = builder.build(user, authenticationState);
142+
final Response response = builder.build(user, authenticationState, request.getAttributes());
143143
listener.onResponse(
144144
new SamlInitiateSingleSignOnResponse(
145145
user.getServiceProvider().getEntityId(),

0 commit comments

Comments
 (0)