mirrors
/
elasticsearch
mirror of https://mirror.osredm.com/root/elasticsearch.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add "extension" attribute validation to IdP SPs (#128805) 

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.
This commit is contained in:
Tim Vernum 2025-06-06 13:09:31 +10:00 committed by GitHub
parent 496fb2d5a4
commit 40cf2d3560
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 143 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
docs/changelog/128805.yaml Normal file
View File

@ -0,0 +1,5 @@
pr: 128805
summary: Add "extension" attribute validation to IdP SPs
area: IdentityProvider
type: enhancement
issues: []

1
server/src/main/java/org/elasticsearch/TransportVersions.java
View File

@ -284,6 +284,7 @@ public class TransportVersions {
public static final TransportVersion JOIN_ON_ALIASES = def(9_088_0_00);
public static final TransportVersion ILM_ADD_SKIP_SETTING = def(9_089_0_00);
public static final TransportVersion ML_INFERENCE_MISTRAL_CHAT_COMPLETION_ADDED = def(9_090_0_00);
public static final TransportVersion IDP_CUSTOM_SAML_ATTRIBUTES_ALLOW_LIST = def(9_091_0_00);
/*
* STOP! READ THIS FIRST! No, really,

3
x-pack/plugin/core/template-resources/src/main/resources/idp/saml-service-provider-template.json
View File

@ -75,6 +75,9 @@
},
"roles": {
"type": "keyword"
},
"extensions": {
"type": "keyword"
}
}
},

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

@ -147,7 +147,8 @@ public class IdentityProviderAuthenticationIT extends IdpRestTestCase {
Map.entry("principal", "https://idp.test.es.elasticsearch.org/attribute/principal"),
Map.entry("name", "https://idp.test.es.elasticsearch.org/attribute/name"),
Map.entry("email", "https://idp.test.es.elasticsearch.org/attribute/email"),
Map.entry("roles", "https://idp.test.es.elasticsearch.org/attribute/roles")
Map.entry("roles", "https://idp.test.es.elasticsearch.org/attribute/roles"),
Map.entry("extensions", List.of("department", "region"))
)
)
);

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

@ -48,6 +48,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
import static org.opensaml.saml.saml2.core.NameIDType.TRANSIENT;
/**
@ -214,28 +215,42 @@ public class SuccessfulAuthenticationResponseMessageBuilder {
final SamlServiceProvider serviceProvider = user.getServiceProvider();
final AttributeStatement statement = samlFactory.object(AttributeStatement.class, AttributeStatement.DEFAULT_ELEMENT_NAME);
final List<Attribute> attributes = new ArrayList<>();
final Attribute roles = buildAttribute(serviceProvider.getAttributeNames().roles, "roles", user.getRoles());
final SamlServiceProvider.AttributeNames attributeNames = serviceProvider.getAttributeNames();
final Attribute roles = buildAttribute(attributeNames.roles, "roles", user.getRoles());
if (roles != null) {
attributes.add(roles);
}
final Attribute principal = buildAttribute(serviceProvider.getAttributeNames().principal, "principal", user.getPrincipal());
final Attribute principal = buildAttribute(attributeNames.principal, "principal", user.getPrincipal());
if (principal != null) {
attributes.add(principal);
}
final Attribute email = buildAttribute(serviceProvider.getAttributeNames().email, "email", user.getEmail());
final Attribute email = buildAttribute(attributeNames.email, "email", user.getEmail());
if (email != null) {
attributes.add(email);
}
final Attribute name = buildAttribute(serviceProvider.getAttributeNames().name, "name", user.getName());
final Attribute name = buildAttribute(attributeNames.name, "name", user.getName());
if (name != null) {
attributes.add(name);
}
// Add custom attributes if provided
if (customAttributes != null && customAttributes.getAttributes().isEmpty() == false) {
for (Map.Entry<String, List<String>> entry : customAttributes.getAttributes().entrySet()) {
Attribute attribute = buildAttribute(entry.getKey(), null, entry.getValue());
if (attribute != null) {
attributes.add(attribute);
final String attributeName = entry.getKey();
if (attributeNames.isAllowedExtension(attributeName)) {
Attribute attribute = buildAttribute(attributeName, null, entry.getValue());
if (attribute != null) {
attributes.add(attribute);
}
} else {
throw new IllegalArgumentException(
"the custom attribute ["
+ attributeName
+ "] is not permitted for the Service Provider ["
+ serviceProvider.getName()
+ "], allowed attribute names are ["
+ collectionToCommaDelimitedString(attributeNames.allowedExtensions)
+ "]"
);
}
}
}

8
x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/sp/SamlServiceProvider.java
View File

@ -36,12 +36,18 @@ public interface SamlServiceProvider {
public final String name;
public final String email;
public final String roles;
public final Set<String> allowedExtensions;
public AttributeNames(String principal, String name, String email, String roles) {
public AttributeNames(String principal, String name, String email, String roles, Set<String> allowedExtensions) {
this.principal = principal;
this.name = name;
this.email = email;
this.roles = roles;
this.allowedExtensions = allowedExtensions == null ? Set.of() : Set.copyOf(allowedExtensions);
}
public boolean isAllowedExtension(String attributeName) {
return this.allowedExtensions.contains(attributeName);
}
}

28
x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/sp/SamlServiceProviderDocument.java
View File

@ -42,6 +42,8 @@ import java.util.SortedSet;
import java.util.TreeSet;
import java.util.function.BiConsumer;
import static org.elasticsearch.TransportVersions.IDP_CUSTOM_SAML_ATTRIBUTES_ALLOW_LIST;
/**
* This class models the storage of a {@link SamlServiceProvider} as an Elasticsearch document.
*/
@ -87,6 +89,12 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
@Nullable
public String roles;
/**
* Extensions are attributes that are provided at runtime (by the trusted client that initiates
* the SAML SSO. They are sourced from the rest request rather than the user object itself.
*/
public Set<String> extensions = Set.of();
public void setPrincipal(String principal) {
this.principal = principal;
}
@ -103,6 +111,10 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
this.roles = roles;
}
public void setExtensions(Collection<String> names) {
this.extensions = names == null ? Set.of() : Set.copyOf(names);
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
@ -111,7 +123,8 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
return Objects.equals(principal, that.principal)
&& Objects.equals(email, that.email)
&& Objects.equals(name, that.name)
&& Objects.equals(roles, that.roles);
&& Objects.equals(roles, that.roles)
&& Objects.equals(extensions, that.extensions);
}
@Override
@ -263,6 +276,10 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
attributeNames.name = in.readOptionalString();
attributeNames.roles = in.readOptionalString();
if (in.getTransportVersion().onOrAfter(IDP_CUSTOM_SAML_ATTRIBUTES_ALLOW_LIST)) {
attributeNames.extensions = in.readCollectionAsImmutableSet(StreamInput::readString);
}
certificates.serviceProviderSigning = in.readStringCollectionAsList();
certificates.identityProviderSigning = in.readStringCollectionAsList();
certificates.identityProviderMetadataSigning = in.readStringCollectionAsList();
@ -288,6 +305,10 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
out.writeOptionalString(attributeNames.name);
out.writeOptionalString(attributeNames.roles);
if (out.getTransportVersion().onOrAfter(IDP_CUSTOM_SAML_ATTRIBUTES_ALLOW_LIST)) {
out.writeStringCollection(attributeNames.extensions);
}
out.writeStringCollection(certificates.serviceProviderSigning);
out.writeStringCollection(certificates.identityProviderSigning);
out.writeStringCollection(certificates.identityProviderMetadataSigning);
@ -426,6 +447,7 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
ATTRIBUTES_PARSER.declareStringOrNull(AttributeNames::setEmail, Fields.Attributes.EMAIL);
ATTRIBUTES_PARSER.declareStringOrNull(AttributeNames::setName, Fields.Attributes.NAME);
ATTRIBUTES_PARSER.declareStringOrNull(AttributeNames::setRoles, Fields.Attributes.ROLES);
ATTRIBUTES_PARSER.declareStringArray(AttributeNames::setExtensions, Fields.Attributes.EXTENSIONS);
DOC_PARSER.declareObject(NULL_CONSUMER, (p, doc) -> CERTIFICATES_PARSER.parse(p, doc.certificates, null), Fields.CERTIFICATES);
CERTIFICATES_PARSER.declareStringArray(Certificates::setServiceProviderSigning, Fields.Certificates.SP_SIGNING);
@ -516,6 +538,9 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
builder.field(Fields.Attributes.EMAIL.getPreferredName(), attributeNames.email);
builder.field(Fields.Attributes.NAME.getPreferredName(), attributeNames.name);
builder.field(Fields.Attributes.ROLES.getPreferredName(), attributeNames.roles);
if (attributeNames.extensions != null && attributeNames.extensions.isEmpty() == false) {
builder.field(Fields.Attributes.EXTENSIONS.getPreferredName(), attributeNames.extensions);
}
builder.endObject();
builder.startObject(Fields.CERTIFICATES.getPreferredName());
@ -553,6 +578,7 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable
ParseField EMAIL = new ParseField("email");
ParseField NAME = new ParseField("name");
ParseField ROLES = new ParseField("roles");
ParseField EXTENSIONS = new ParseField("extensions");
}
interface Certificates {

3
x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/sp/SamlServiceProviderFactory.java
View File

@ -38,7 +38,8 @@ public final class SamlServiceProviderFactory {
document.attributeNames.principal,
document.attributeNames.name,
document.attributeNames.email,
document.attributeNames.roles
document.attributeNames.roles,
document.attributeNames.extensions
);
final Set<X509Credential> credentials = document.certificates.getServiceProviderX509SigningCertificates()
.stream()

21
x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/sp/SamlServiceProviderIndex.java
View File

@ -81,7 +81,26 @@ public class SamlServiceProviderIndex implements Closeable {
static final String TEMPLATE_VERSION_STRING_DEPRECATED = "idp.template.version_deprecated";
static final String FINAL_TEMPLATE_VERSION_STRING_DEPRECATED = "8.14.0";
static final int CURRENT_TEMPLATE_VERSION = 1;
/**
* The object in the index mapping metadata that contains a version field
*/
private static final String INDEX_META_FIELD = "_meta";
/**
* The field in the {@link #INDEX_META_FIELD} metadata that contains the version number
*/
private static final String TEMPLATE_VERSION_META_FIELD = "idp-template-version";
/**
* The first version of this template (since it was moved to use {@link org.elasticsearch.xpack.core.template.IndexTemplateRegistry}
*/
private static final int VERSION_ORIGINAL = 1;
/**
* The version that added the {@code attributes.extensions} field to the SAML SP document
*/
private static final int VERSION_EXTENSION_ATTRIBUTES = 2;
static final int CURRENT_TEMPLATE_VERSION = VERSION_EXTENSION_ATTRIBUTES;
private volatile boolean indexUpToDate = false;
public static final class DocumentVersion {
public final String id;

3
x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/action/TransportSamlInitiateSingleSignOnActionTests.java
View File

@ -190,7 +190,8 @@ public class TransportSamlInitiateSingleSignOnActionTests extends IdpSamlTestCas
"https://saml.elasticsearch.org/attributes/principal",
"https://saml.elasticsearch.org/attributes/name",
"https://saml.elasticsearch.org/attributes/email",
"https://saml.elasticsearch.org/attributes/roles"
"https://saml.elasticsearch.org/attributes/roles",
Set.of()
),
null,
false,

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

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.idp.saml.authn;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.xpack.idp.saml.idp.SamlIdentityProvider;
import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProvider;
import org.elasticsearch.xpack.idp.saml.sp.ServiceProviderDefaults;
@ -103,7 +104,46 @@ public class SuccessfulAuthenticationResponseMessageBuilderTests extends IdpSaml
assertTrue("Custom attribute 'customAttr2' not found in SAML response", foundCustomAttr2);
}
private Response buildResponse(SamlInitiateSingleSignOnAttributes customAttributes) throws Exception {
public void testRejectInvalidCustomAttributes() throws Exception {
final var customAttributes = new SamlInitiateSingleSignOnAttributes(
Map.of("https://idp.example.org/attribute/department", Collections.singletonList("engineering"))
);
// Build response with custom attributes
final IllegalArgumentException ex = expectThrows(
IllegalArgumentException.class,
() -> buildResponse(
new SamlServiceProvider.AttributeNames(
"https://idp.example.org/attribute/principal",
null,
null,
null,
Set.of("https://idp.example.org/attribute/avatar")
),
customAttributes
)
);
assertThat(ex.getMessage(), containsString("custom attribute [https://idp.example.org/attribute/department]"));
assertThat(ex.getMessage(), containsString("allowed attribute names are [https://idp.example.org/attribute/avatar]"));
}
private Response buildResponse(@Nullable SamlInitiateSingleSignOnAttributes customAttributes) throws Exception {
return buildResponse(
new SamlServiceProvider.AttributeNames(
"principal",
null,
null,
null,
customAttributes == null ? Set.of() : customAttributes.getAttributes().keySet()
),
customAttributes
);
}
private Response buildResponse(
final SamlServiceProvider.AttributeNames attributes,
@Nullable SamlInitiateSingleSignOnAttributes customAttributes
) throws Exception {
final Clock clock = Clock.systemUTC();
final SamlServiceProvider sp = mock(SamlServiceProvider.class);
@ -112,7 +152,7 @@ public class SuccessfulAuthenticationResponseMessageBuilderTests extends IdpSaml
when(sp.getEntityId()).thenReturn(baseServiceUrl);
when(sp.getAssertionConsumerService()).thenReturn(URI.create(acs).toURL());
when(sp.getAuthnExpiry()).thenReturn(Duration.ofMinutes(10));
when(sp.getAttributeNames()).thenReturn(new SamlServiceProvider.AttributeNames("principal", null, null, null));
when(sp.getAttributeNames()).thenReturn(attributes);
final UserServiceAuthentication user = mock(UserServiceAuthentication.class);
when(user.getPrincipal()).thenReturn(randomAlphaOfLengthBetween(4, 12));

11
x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/sp/WildcardServiceProviderResolverTests.java
View File

@ -24,6 +24,7 @@ import java.util.Map;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@ -82,7 +83,8 @@ public class WildcardServiceProviderResolverTests extends IdpSamlTestCase {
"principal": "http://cloud.elastic.co/saml/principal",
"name": "http://cloud.elastic.co/saml/name",
"email": "http://cloud.elastic.co/saml/email",
"roles": "http://cloud.elastic.co/saml/roles"
"roles": "http://cloud.elastic.co/saml/roles",
"extensions": [ "http://cloud.elastic.co/saml/department", "http://cloud.elastic.co/saml/avatar" ]
}
}
}
@ -154,6 +156,7 @@ public class WildcardServiceProviderResolverTests extends IdpSamlTestCase {
assertThat(sp1.getAssertionConsumerService().toString(), equalTo("https://abcdef.service.example.com/saml2/acs"));
assertThat(sp1.getName(), equalTo("abcdef at example.com (A)"));
assertThat(sp1.getPrivileges().getResource(), equalTo("service1:example:abcdef"));
assertThat(sp1.getAttributeNames().allowedExtensions, empty());
final SamlServiceProvider sp2 = resolver.resolve("https://qwerty.example.com/", "https://qwerty.service.example.com/saml2/acs");
assertThat(sp2, notNullValue());
@ -161,6 +164,7 @@ public class WildcardServiceProviderResolverTests extends IdpSamlTestCase {
assertThat(sp2.getAssertionConsumerService().toString(), equalTo("https://qwerty.service.example.com/saml2/acs"));
assertThat(sp2.getName(), equalTo("qwerty at example.com (A)"));
assertThat(sp2.getPrivileges().getResource(), equalTo("service1:example:qwerty"));
assertThat(sp2.getAttributeNames().allowedExtensions, empty());
final SamlServiceProvider sp3 = resolver.resolve("https://xyzzy.example.com/", "https://services.example.com/xyzzy/saml2/acs");
assertThat(sp3, notNullValue());
@ -168,6 +172,7 @@ public class WildcardServiceProviderResolverTests extends IdpSamlTestCase {
assertThat(sp3.getAssertionConsumerService().toString(), equalTo("https://services.example.com/xyzzy/saml2/acs"));
assertThat(sp3.getName(), equalTo("xyzzy at example.com (B)"));
assertThat(sp3.getPrivileges().getResource(), equalTo("service1:example:xyzzy"));
assertThat(sp3.getAttributeNames().allowedExtensions, empty());
final SamlServiceProvider sp4 = resolver.resolve("https://service-12345.example.net/", "https://saml.example.net/12345/acs");
assertThat(sp4, notNullValue());
@ -175,6 +180,10 @@ public class WildcardServiceProviderResolverTests extends IdpSamlTestCase {
assertThat(sp4.getAssertionConsumerService().toString(), equalTo("https://saml.example.net/12345/acs"));
assertThat(sp4.getName(), equalTo("12345 at example.net"));
assertThat(sp4.getPrivileges().getResource(), equalTo("service2:example:12345"));
assertThat(
sp4.getAttributeNames().allowedExtensions,
containsInAnyOrder("http://cloud.elastic.co/saml/department", "http://cloud.elastic.co/saml/avatar")
);
expectThrows(
IllegalArgumentException.class,