Skip to content

Commit 04e8c45

Browse files
committed
* Remove friendly name.
* Make attributes map final in SamlInitiateSingleSignOnAttributes Signed-off-by: lloydmeta <[email protected]>
1 parent be900e6 commit 04e8c45

File tree

5 files changed

+29
-60
lines changed

5 files changed

+29
-60
lines changed

x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SuccessfulAuthenticationResponseMessageBuilder.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ private AttributeStatement buildAttributes(
233233
// Add custom attributes if provided
234234
if (customAttributes != null && customAttributes.getAttributes().isEmpty() == false) {
235235
for (Map.Entry<String, List<String>> entry : customAttributes.getAttributes().entrySet()) {
236-
Attribute attribute = buildAttribute(entry.getKey(), entry.getKey(), entry.getValue());
236+
Attribute attribute = buildAttribute(entry.getKey(), null, entry.getValue());
237237
if (attribute != null) {
238238
attributes.add(attribute);
239239
}
@@ -247,20 +247,22 @@ private AttributeStatement buildAttributes(
247247
return statement;
248248
}
249249

250-
private Attribute buildAttribute(String formalName, String friendlyName, String value) {
250+
private Attribute buildAttribute(String formalName, @Nullable String friendlyName, String value) {
251251
if (Strings.isNullOrEmpty(value)) {
252252
return null;
253253
}
254254
return buildAttribute(formalName, friendlyName, List.of(value));
255255
}
256256

257-
private Attribute buildAttribute(String formalName, String friendlyName, Collection<String> values) {
257+
private Attribute buildAttribute(String formalName, @Nullable String friendlyName, Collection<String> values) {
258258
if (values.isEmpty() || Strings.isNullOrEmpty(formalName)) {
259259
return null;
260260
}
261261
final Attribute attribute = samlFactory.object(Attribute.class, Attribute.DEFAULT_ELEMENT_NAME);
262262
attribute.setName(formalName);
263-
attribute.setFriendlyName(friendlyName);
263+
if (Strings.isNullOrEmpty(friendlyName) == false) {
264+
attribute.setFriendlyName(friendlyName);
265+
}
264266
attribute.setNameFormat(Attribute.URI_REFERENCE);
265267
for (String val : values) {
266268
final XSString string = samlFactory.object(XSString.class, AttributeValue.DEFAULT_ELEMENT_NAME, XSString.TYPE_NAME);

x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlInitiateSingleSignOnAttributes.java

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
* Each attribute has a key and a list of values.
3030
*/
3131
public class SamlInitiateSingleSignOnAttributes implements Writeable, ToXContentObject {
32-
private Map<String, List<String>> attributes;
32+
private final Map<String, List<String>> attributes;
3333

34-
public SamlInitiateSingleSignOnAttributes() {
35-
this.attributes = new HashMap<>();
34+
public SamlInitiateSingleSignOnAttributes(Map<String, List<String>> attributes) {
35+
this.attributes = attributes;
3636
}
3737

3838
/**
@@ -42,11 +42,6 @@ public Map<String, List<String>> getAttributes() {
4242
return Collections.unmodifiableMap(attributes);
4343
}
4444

45-
public SamlInitiateSingleSignOnAttributes setAttributes(Map<String, List<String>> attributes) {
46-
this.attributes = new HashMap<>(attributes);
47-
return this;
48-
}
49-
5045
/**
5146
* Creates a SamlInitiateSingleSignOnAttributes object by parsing the provided JSON content.
5247
* Expects a JSON structure like: { "attr1": ["val1", "val2"], "attr2": ["val3"] }
@@ -66,9 +61,7 @@ public static SamlInitiateSingleSignOnAttributes fromXContent(XContentParser par
6661
attributes.put(key, values);
6762
}
6863
}
69-
SamlInitiateSingleSignOnAttributes attributesObj = new SamlInitiateSingleSignOnAttributes();
70-
attributesObj.setAttributes(attributes);
71-
return attributesObj;
64+
return new SamlInitiateSingleSignOnAttributes(attributes);
7265
}
7366

7467
@Override

x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/action/SamlInitiateSingleSignOnRequestTests.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,21 @@ public void testDuplicateAttributeKeysValidation() {
5454
request.setAssertionConsumerService("https://kibana_url/acs");
5555

5656
// Test with valid attribute keys
57-
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
5857
Map<String, List<String>> attributeMap = new HashMap<>();
5958
attributeMap.put("key1", Collections.singletonList("value1"));
6059
attributeMap.put("key2", Arrays.asList("value2A", "value2B"));
61-
attributes.setAttributes(attributeMap);
60+
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
6261
request.setAttributes(attributes);
6362

6463
// Should pass validation
6564
ActionRequestValidationException validationException = request.validate();
6665
assertNull("Request with valid attribute keys should pass validation", validationException);
6766

6867
// Test with empty attribute key - should be invalid
69-
attributes = new SamlInitiateSingleSignOnAttributes();
7068
attributeMap = new HashMap<>();
7169
attributeMap.put("", Collections.singletonList("value1"));
7270
attributeMap.put("unique_key", Collections.singletonList("value2"));
73-
attributes.setAttributes(attributeMap);
71+
attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
7472
request.setAttributes(attributes);
7573

7674
// Should fail validation with appropriate error message

x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/authn/SuccessfulAuthenticationResponseMessageBuilderTests.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,14 @@ public void testSignedResponseIsValidAgainstXmlSchema() throws Exception {
6262

6363
public void testSignedResponseWithCustomAttributes() throws Exception {
6464
// Create custom attributes
65-
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
6665
Map<String, List<String>> attributeMap = new HashMap<>();
6766
attributeMap.put("customAttr1", Collections.singletonList("value1"));
6867

6968
List<String> multipleValues = new ArrayList<>();
7069
multipleValues.add("value2A");
7170
multipleValues.add("value2B");
7271
attributeMap.put("customAttr2", multipleValues);
73-
74-
attributes.setAttributes(attributeMap);
72+
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
7573

7674
// Build response with custom attributes
7775
final Response response = buildResponse(attributes);

x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/support/SamlInitiateSingleSignOnAttributesTests.java

Lines changed: 16 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -35,24 +35,18 @@
3535
public class SamlInitiateSingleSignOnAttributesTests extends ESTestCase {
3636

3737
public void testConstructors() throws Exception {
38-
// Test default constructor
39-
final SamlInitiateSingleSignOnAttributes attributes1 = new SamlInitiateSingleSignOnAttributes();
38+
final SamlInitiateSingleSignOnAttributes attributes1 = new SamlInitiateSingleSignOnAttributes(Collections.emptyMap());
4039
assertThat(attributes1.getAttributes(), Matchers.anEmptyMap());
4140

42-
// Test a second instance is also empty (not holding state)
43-
final SamlInitiateSingleSignOnAttributes attributes2 = new SamlInitiateSingleSignOnAttributes();
44-
assertThat(attributes2.getAttributes(), Matchers.anEmptyMap());
45-
4641
// Test adding attributes
4742
Map<String, List<String>> attributeMap = new HashMap<>();
4843
attributeMap.put("key1", Collections.singletonList("value1"));
49-
final SamlInitiateSingleSignOnAttributes attributes3 = new SamlInitiateSingleSignOnAttributes();
50-
attributes3.setAttributes(attributeMap);
44+
final SamlInitiateSingleSignOnAttributes attributes3 = new SamlInitiateSingleSignOnAttributes(attributeMap);
5145
assertThat(attributes3.getAttributes().size(), equalTo(1));
5246
}
5347

5448
public void testEmptyAttributes() throws Exception {
55-
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
49+
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(Collections.emptyMap());
5650

5751
// Test toXContent
5852
XContentBuilder builder = XContentFactory.jsonBuilder();
@@ -70,12 +64,10 @@ public void testEmptyAttributes() throws Exception {
7064
}
7165

7266
public void testWithAttributes() throws Exception {
73-
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
74-
7567
Map<String, List<String>> attributeMap = new HashMap<>();
7668
attributeMap.put("key1", Arrays.asList("value1", "value2"));
7769
attributeMap.put("key2", Collections.singletonList("value3"));
78-
attributes.setAttributes(attributeMap);
70+
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
7971

8072
// Test getAttributes
8173
Map<String, List<String>> returnedAttributes = attributes.getAttributes();
@@ -121,38 +113,28 @@ public void testWithAttributes() throws Exception {
121113
}
122114

123115
public void testToString() {
124-
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
125116
Map<String, List<String>> attributeMap = new HashMap<>();
126117
attributeMap.put("key1", Arrays.asList("value1", "value2"));
127-
attributes.setAttributes(attributeMap);
118+
final SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
128119

129120
String toString = attributes.toString();
130121
assertThat(toString, containsString("SamlInitiateSingleSignOnAttributes"));
131122
assertThat(toString, containsString("key1"));
132123
assertThat(toString, containsString("value1"));
133124
assertThat(toString, containsString("value2"));
134125

135-
// Add another attribute
136-
attributeMap.put("key2", Collections.singletonList("value3"));
137-
attributes.setAttributes(attributeMap);
138-
139-
toString = attributes.toString();
140-
assertThat(toString, containsString("key2"));
141-
assertThat(toString, containsString("value3"));
142-
143126
// Test empty attributes
144-
final SamlInitiateSingleSignOnAttributes emptyAttributes = new SamlInitiateSingleSignOnAttributes();
127+
final SamlInitiateSingleSignOnAttributes emptyAttributes = new SamlInitiateSingleSignOnAttributes(Collections.emptyMap());
145128
toString = emptyAttributes.toString();
146129
assertThat(toString, containsString("SamlInitiateSingleSignOnAttributes"));
147130
assertThat(toString, containsString("attributes={}"));
148131
}
149132

150133
public void testValidation() throws Exception {
151134
// Test validation with empty key
152-
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes();
153135
Map<String, List<String>> attributeMap = new HashMap<>();
154136
attributeMap.put("", Arrays.asList("value1", "value2"));
155-
attributes.setAttributes(attributeMap);
137+
SamlInitiateSingleSignOnAttributes attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
156138

157139
ActionRequestValidationException validationException = attributes.validate();
158140
assertNotNull(validationException);
@@ -161,7 +143,7 @@ public void testValidation() throws Exception {
161143
// Test validation with null key
162144
attributeMap = new HashMap<>();
163145
attributeMap.put(null, Collections.singletonList("value"));
164-
attributes.setAttributes(attributeMap);
146+
attributes = new SamlInitiateSingleSignOnAttributes(attributeMap);
165147

166148
validationException = attributes.validate();
167149
assertNotNull(validationException);
@@ -173,19 +155,17 @@ public void testEqualsAndHashCode() {
173155
attributeMap1.put("key1", Arrays.asList("value1", "value2"));
174156
attributeMap1.put("key2", Collections.singletonList("value3"));
175157

176-
SamlInitiateSingleSignOnAttributes attributes1 = new SamlInitiateSingleSignOnAttributes();
177-
attributes1.setAttributes(attributeMap1);
158+
SamlInitiateSingleSignOnAttributes attributes1 = new SamlInitiateSingleSignOnAttributes(attributeMap1);
178159

179160
Map<String, List<String>> attributeMap2 = new HashMap<>();
180161
attributeMap2.put("key1", Arrays.asList("value1", "value2"));
181162
attributeMap2.put("key2", Collections.singletonList("value3"));
182163

183-
SamlInitiateSingleSignOnAttributes attributes2 = new SamlInitiateSingleSignOnAttributes();
184-
attributes2.setAttributes(attributeMap2);
164+
SamlInitiateSingleSignOnAttributes attributes2 = new SamlInitiateSingleSignOnAttributes(attributeMap2);
185165

186166
// Test equals
187-
assertTrue(attributes1.equals(attributes2));
188-
assertTrue(attributes2.equals(attributes1));
167+
assertEquals(attributes1, attributes2);
168+
assertEquals(attributes2, attributes1);
189169

190170
// Test hashCode
191171
assertThat(attributes1.hashCode(), equalTo(attributes2.hashCode()));
@@ -195,19 +175,17 @@ public void testEqualsAndHashCode() {
195175
attributeMap3.put("key1", Arrays.asList("different", "value2"));
196176
attributeMap3.put("key2", Collections.singletonList("value3"));
197177

198-
SamlInitiateSingleSignOnAttributes attributes3 = new SamlInitiateSingleSignOnAttributes();
199-
attributes3.setAttributes(attributeMap3);
178+
SamlInitiateSingleSignOnAttributes attributes3 = new SamlInitiateSingleSignOnAttributes(attributeMap3);
200179

201-
assertFalse(attributes1.equals(attributes3));
180+
assertNotEquals(attributes1, attributes3);
202181

203182
// Test with missing key
204183
Map<String, List<String>> attributeMap4 = new HashMap<>();
205184
attributeMap4.put("key1", Arrays.asList("value1", "value2"));
206185

207-
SamlInitiateSingleSignOnAttributes attributes4 = new SamlInitiateSingleSignOnAttributes();
208-
attributes4.setAttributes(attributeMap4);
186+
SamlInitiateSingleSignOnAttributes attributes4 = new SamlInitiateSingleSignOnAttributes(attributeMap4);
209187

210-
assertFalse(attributes1.equals(attributes4));
188+
assertNotEquals(attributes1, attributes4);
211189
}
212190

213191
private SamlInitiateSingleSignOnAttributes parseFromJson(String json) throws IOException {

0 commit comments

Comments
 (0)