From 077b47db3703323fe4cdfa07ba7dbfda3380c491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Fred=C3=A9n?= <109296772+jfreden@users.noreply.github.com> Date: Thu, 7 Dec 2023 08:55:46 +0100 Subject: [PATCH] Add support for splitting saml groups by delimiter (#102769) * Add support for splitting saml groups by delimiter --- .../settings/security-settings.asciidoc | 14 ++ .../authc/saml/SamlRealmSettings.java | 39 +++- .../xpack/security/authc/saml/SamlRealm.java | 62 +++++- .../security/authc/saml/SamlRealmTests.java | 183 ++++++++++++++++-- 4 files changed, 284 insertions(+), 14 deletions(-) diff --git a/docs/reference/settings/security-settings.asciidoc b/docs/reference/settings/security-settings.asciidoc index 5cf55b8434f9d..5729a31b6c728 100644 --- a/docs/reference/settings/security-settings.asciidoc +++ b/docs/reference/settings/security-settings.asciidoc @@ -1253,6 +1253,20 @@ As per `attribute_patterns.principal`, but for the _mail_ property. As per `attribute_patterns.principal`, but for the _dn_ property. // end::saml-attributes-patterns-dn-tag[] +// tag::saml-attributes-delimiters-groups-tag[] +`attribute_delimiters.groups` {ess-icon}:: +(<>) +A plain string that is used as a delimiter to split a single-valued SAML +attribute specified by attributes.groups before it is applied to the user's +groups property. For example, splitting the SAML attribute value +engineering,elasticsearch-admins,employees on a delimiter value of , will +result in engineering, elasticsearch-admins, and employees as the list of +groups for the user. The delimiter will always be split on, regardless of +escaping in the input string. This setting does not support multi-valued SAML +attributes. It cannot be used together with the attribute_patterns setting. +You can only configure this setting for the groups attribute. +// end::saml-attributes-delimiters-groups-tag[] + // tag::saml-nameid-format-tag[] `nameid_format` {ess-icon}:: (<>) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java index ea3cd08dbc4ff..83d197e78f583 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java @@ -15,6 +15,7 @@ import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings; import org.elasticsearch.xpack.core.ssl.X509KeyPairSettings; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -102,7 +103,7 @@ public class SamlRealmSettings { ); public static final AttributeSetting PRINCIPAL_ATTRIBUTE = new AttributeSetting("principal"); - public static final AttributeSetting GROUPS_ATTRIBUTE = new AttributeSetting("groups"); + public static final AttributeSettingWithDelimiter GROUPS_ATTRIBUTE = new AttributeSettingWithDelimiter("groups"); public static final AttributeSetting DN_ATTRIBUTE = new AttributeSetting("dn"); public static final AttributeSetting NAME_ATTRIBUTE = new AttributeSetting("name"); public static final AttributeSetting MAIL_ATTRIBUTE = new AttributeSetting("mail"); @@ -221,4 +222,40 @@ public Setting.AffixSetting getPattern() { return pattern; } } + + /** + * The SAML realm offers a setting where a multivalued attribute can be configured to have a delimiter for its values, for the case + * when all values are provided in a single string item, separated by a delimiter. + * As in {@link AttributeSetting} there are two settings: + *
    + *
  • The name of the SAML attribute to use
    • + *
  • A delimiter to apply to that attribute value in order to extract the substrings that should be used.
    • + *
+ * For example, the Elasticsearch Group could be configured to come from the SAML "department" attribute, where all groups are provided + * as a csv value in a single list item. + */ + public static final class AttributeSettingWithDelimiter { + public static final String ATTRIBUTE_DELIMITERS_PREFIX = "attribute_delimiters."; + private final Setting.AffixSetting delimiter; + private final AttributeSetting attributeSetting; + + public AttributeSetting getAttributeSetting() { + return attributeSetting; + } + + public AttributeSettingWithDelimiter(String name) { + this.attributeSetting = new AttributeSetting(name); + this.delimiter = RealmSettings.simpleString(TYPE, ATTRIBUTE_DELIMITERS_PREFIX + name, Setting.Property.NodeScope); + } + + public Setting.AffixSetting getDelimiter() { + return this.delimiter; + } + + public Collection> settings() { + List> settings = new ArrayList<>(attributeSetting.settings()); + settings.add(getDelimiter()); + return settings; + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java index 856fdc4ba9555..56446907ad8f6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java @@ -278,7 +278,7 @@ public SpConfiguration getServiceProvider() { this.populateUserMetadata = config.getSetting(POPULATE_USER_METADATA); this.principalAttribute = AttributeParser.forSetting(logger, PRINCIPAL_ATTRIBUTE, config, true); - this.groupsAttribute = AttributeParser.forSetting(logger, GROUPS_ATTRIBUTE, config, false); + this.groupsAttribute = AttributeParser.forSetting(logger, GROUPS_ATTRIBUTE, config); this.dnAttribute = AttributeParser.forSetting(logger, DN_ATTRIBUTE, config, false); this.nameAttribute = AttributeParser.forSetting(logger, NAME_ATTRIBUTE, config, false); this.mailAttribute = AttributeParser.forSetting(logger, MAIL_ATTRIBUTE, config, false); @@ -1004,6 +1004,66 @@ public String toString() { return name; } + static AttributeParser forSetting(Logger logger, SamlRealmSettings.AttributeSettingWithDelimiter setting, RealmConfig realmConfig) { + SamlRealmSettings.AttributeSetting attributeSetting = setting.getAttributeSetting(); + if (realmConfig.hasSetting(setting.getDelimiter())) { + if (realmConfig.hasSetting(attributeSetting.getAttribute()) == false) { + throw new SettingsException( + "Setting [" + + RealmSettings.getFullSettingKey(realmConfig, setting.getDelimiter()) + + "] cannot be set unless [" + + RealmSettings.getFullSettingKey(realmConfig, attributeSetting.getAttribute()) + + "] is also set" + ); + } + if (realmConfig.hasSetting(attributeSetting.getPattern())) { + throw new SettingsException( + "Setting [" + + RealmSettings.getFullSettingKey(realmConfig, attributeSetting.getPattern()) + + "] can not be set when [" + + RealmSettings.getFullSettingKey(realmConfig, setting.getDelimiter()) + + "] is set" + ); + } + + String attributeName = realmConfig.getSetting(attributeSetting.getAttribute()); + String delimiter = realmConfig.getSetting(setting.getDelimiter()); + return new AttributeParser( + "SAML Attribute [" + + attributeName + + "] with delimiter [" + + delimiter + + "] for [" + + attributeSetting.name(realmConfig) + + "]", + attributes -> { + List attributeValues = attributes.getAttributeValues(attributeName); + if (attributeValues.size() > 1) { + throw SamlUtils.samlException( + "Expected single string value for attribute: [" + + attributeName + + "], but got list with " + + attributeValues.size() + + " values" + ); + } + return attributeValues.stream() + .map(s -> s.split(Pattern.quote(delimiter))) + .flatMap(Arrays::stream) + .filter(attribute -> { + if (Strings.isNullOrEmpty(attribute)) { + logger.debug("Attribute [{}] has empty components when using delimiter [{}]", attributeName, delimiter); + return false; + } + return true; + }) + .collect(Collectors.toList()); + } + ); + } + return AttributeParser.forSetting(logger, attributeSetting, realmConfig, false); + } + static AttributeParser forSetting( Logger logger, SamlRealmSettings.AttributeSetting setting, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java index f46015106d204..3ba9b18b24036 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java @@ -352,14 +352,30 @@ public void testAuthenticateWithRoleMapping() throws Exception { final boolean principalIsEmailAddress = randomBoolean(); final Boolean populateUserMetadata = randomFrom(Boolean.TRUE, Boolean.FALSE, null); final String authenticatingRealm = randomBoolean() ? REALM_NAME : null; - AuthenticationResult result = performAuthentication( - roleMapper, - useNameId, - principalIsEmailAddress, - populateUserMetadata, - false, - authenticatingRealm - ); + final boolean testWithDelimiter = randomBoolean(); + final AuthenticationResult result; + + if (testWithDelimiter) { + result = performAuthentication( + roleMapper, + useNameId, + principalIsEmailAddress, + populateUserMetadata, + false, + authenticatingRealm, + List.of("STRIKE Team: Delta$shield"), + "$" + ); + } else { + result = performAuthentication( + roleMapper, + useNameId, + principalIsEmailAddress, + populateUserMetadata, + false, + authenticatingRealm + ); + } assertThat(result, notNullValue()); assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.SUCCESS)); assertThat(result.getValue().principal(), equalTo(useNameId ? "clint.barton" : "cbarton")); @@ -377,7 +393,11 @@ public void testAuthenticateWithRoleMapping() throws Exception { } assertThat(userData.get().getUsername(), equalTo(useNameId ? "clint.barton" : "cbarton")); - assertThat(userData.get().getGroups(), containsInAnyOrder("avengers", "shield")); + if (testWithDelimiter) { + assertThat(userData.get().getGroups(), containsInAnyOrder("STRIKE Team: Delta", "shield")); + } else { + assertThat(userData.get().getGroups(), containsInAnyOrder("avengers", "shield")); + } } public void testAuthenticateWithAuthorizingRealm() throws Exception { @@ -431,6 +451,28 @@ private AuthenticationResult performAuthentication( Boolean populateUserMetadata, boolean useAuthorizingRealm, String authenticatingRealm + ) throws Exception { + return performAuthentication( + roleMapper, + useNameId, + principalIsEmailAddress, + populateUserMetadata, + useAuthorizingRealm, + authenticatingRealm, + Arrays.asList("avengers", "shield"), + null + ); + } + + private AuthenticationResult performAuthentication( + UserRoleMapper roleMapper, + boolean useNameId, + boolean principalIsEmailAddress, + Boolean populateUserMetadata, + boolean useAuthorizingRealm, + String authenticatingRealm, + List groups, + String groupsDelimiter ) throws Exception { final EntityDescriptor idp = mockIdp(); final SpConfiguration sp = new SpConfiguration("", "https://saml/", null, null, null, Collections.emptyList()); @@ -453,8 +495,12 @@ private AuthenticationResult performAuthentication( final Settings.Builder settingsBuilder = Settings.builder() .put(getFullSettingKey(REALM_NAME, SamlRealmSettings.PRINCIPAL_ATTRIBUTE.getAttribute()), useNameId ? "nameid" : "uid") - .put(getFullSettingKey(REALM_NAME, SamlRealmSettings.GROUPS_ATTRIBUTE.getAttribute()), "groups") + .put(getFullSettingKey(REALM_NAME, SamlRealmSettings.GROUPS_ATTRIBUTE.getAttributeSetting().getAttribute()), "groups") .put(getFullSettingKey(REALM_NAME, SamlRealmSettings.MAIL_ATTRIBUTE.getAttribute()), "mail"); + + if (groupsDelimiter != null) { + settingsBuilder.put(getFullSettingKey(REALM_NAME, SamlRealmSettings.GROUPS_ATTRIBUTE.getDelimiter()), groupsDelimiter); + } if (principalIsEmailAddress) { final boolean anchoredMatch = randomBoolean(); settingsBuilder.put( @@ -497,7 +543,7 @@ private AuthenticationResult performAuthentication( randomAlphaOfLength(16), Arrays.asList( new SamlAttributes.SamlAttribute("urn:oid:0.9.2342.19200300.100.1.1", "uid", Collections.singletonList(uidValue)), - new SamlAttributes.SamlAttribute("urn:oid:1.3.6.1.4.1.5923.1.5.1.1", "groups", Arrays.asList("avengers", "shield")), + new SamlAttributes.SamlAttribute("urn:oid:1.3.6.1.4.1.5923.1.5.1.1", "groups", groups), new SamlAttributes.SamlAttribute("urn:oid:0.9.2342.19200300.100.1.3", "mail", Arrays.asList("cbarton@shield.gov")) ) ); @@ -534,7 +580,120 @@ public SamlRealm buildRealm( } } - public void testAttributeSelectionWithRegex() throws Exception { + public void testAttributeSelectionWithSplit() { + List strings = performAttributeSelectionWithSplit(",", "departments", "engineering", "elasticsearch-admins", "employees"); + assertThat("For attributes: " + strings, strings, contains("engineering", "elasticsearch-admins", "employees")); + } + + public void testAttributeSelectionWithSplitEmptyInput() { + List strings = performAttributeSelectionWithSplit(",", "departments"); + assertThat("For attributes: " + strings, strings, is(empty())); + } + + public void testAttributeSelectionWithSplitJustDelimiter() { + List strings = performAttributeSelectionWithSplit(",", ","); + assertThat("For attributes: " + strings, strings, is(empty())); + } + + public void testAttributeSelectionWithSplitNoDelimiter() { + List strings = performAttributeSelectionWithSplit(",", "departments", "elasticsearch-team"); + assertThat("For attributes: " + strings, strings, contains("elasticsearch-team")); + } + + private List performAttributeSelectionWithSplit(String delimiter, String groupAttributeName, String... returnedGroups) { + final Settings settings = Settings.builder() + .put(REALM_SETTINGS_PREFIX + ".attributes.groups", groupAttributeName) + .put(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups", delimiter) + .build(); + + final RealmConfig config = buildConfig(settings); + + final SamlRealmSettings.AttributeSettingWithDelimiter groupSetting = new SamlRealmSettings.AttributeSettingWithDelimiter("groups"); + final SamlRealm.AttributeParser parser = SamlRealm.AttributeParser.forSetting(logger, groupSetting, config); + + final SamlAttributes attributes = new SamlAttributes( + new SamlNameId(NameIDType.TRANSIENT, randomAlphaOfLength(24), null, null, null), + randomAlphaOfLength(16), + Collections.singletonList( + new SamlAttributes.SamlAttribute( + "departments", + "departments", + Collections.singletonList(String.join(delimiter, returnedGroups)) + ) + ) + ); + return parser.getAttribute(attributes); + } + + public void testAttributeSelectionWithDelimiterAndPatternThrowsSettingsException() throws Exception { + final Settings settings = Settings.builder() + .put(REALM_SETTINGS_PREFIX + ".attributes.groups", "departments") + .put(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups", ",") + .put(REALM_SETTINGS_PREFIX + ".attribute_patterns.groups", "^(.+)@\\w+.example.com$") + .build(); + + final RealmConfig config = buildConfig(settings); + + final SamlRealmSettings.AttributeSettingWithDelimiter groupSetting = new SamlRealmSettings.AttributeSettingWithDelimiter("groups"); + + final SettingsException settingsException = expectThrows( + SettingsException.class, + () -> SamlRealm.AttributeParser.forSetting(logger, groupSetting, config) + ); + + assertThat(settingsException.getMessage(), containsString(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups")); + assertThat(settingsException.getMessage(), containsString(REALM_SETTINGS_PREFIX + ".attribute_patterns.groups")); + } + + public void testAttributeSelectionNoGroupsConfiguredThrowsSettingsException() { + String delimiter = ","; + final Settings settings = Settings.builder().put(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups", delimiter).build(); + final RealmConfig config = buildConfig(settings); + final SamlRealmSettings.AttributeSettingWithDelimiter groupSetting = new SamlRealmSettings.AttributeSettingWithDelimiter("groups"); + + final SettingsException settingsException = expectThrows( + SettingsException.class, + () -> SamlRealm.AttributeParser.forSetting(logger, groupSetting, config) + ); + + assertThat(settingsException.getMessage(), containsString(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups")); + assertThat(settingsException.getMessage(), containsString(REALM_SETTINGS_PREFIX + ".attributes.groups")); + } + + public void testAttributeSelectionWithSplitAndListThrowsSecurityException() { + String delimiter = ","; + + final Settings settings = Settings.builder() + .put(REALM_SETTINGS_PREFIX + ".attributes.groups", "departments") + .put(REALM_SETTINGS_PREFIX + ".attribute_delimiters.groups", delimiter) + .build(); + + final RealmConfig config = buildConfig(settings); + + final SamlRealmSettings.AttributeSettingWithDelimiter groupSetting = new SamlRealmSettings.AttributeSettingWithDelimiter("groups"); + final SamlRealm.AttributeParser parser = SamlRealm.AttributeParser.forSetting(logger, groupSetting, config); + + final SamlAttributes attributes = new SamlAttributes( + new SamlNameId(NameIDType.TRANSIENT, randomAlphaOfLength(24), null, null, null), + randomAlphaOfLength(16), + Collections.singletonList( + new SamlAttributes.SamlAttribute( + "departments", + "departments", + List.of("engineering", String.join(delimiter, "elasticsearch-admins", "employees")) + ) + ) + ); + + ElasticsearchSecurityException securityException = expectThrows( + ElasticsearchSecurityException.class, + () -> parser.getAttribute(attributes) + ); + + assertThat(securityException.getMessage(), containsString("departments")); + } + + public void testAttributeSelectionWithRegex() { final boolean useFriendlyName = randomBoolean(); final Settings settings = Settings.builder() .put(REALM_SETTINGS_PREFIX + ".attributes.principal", useFriendlyName ? "mail" : "urn:oid:0.9.2342.19200300.100.1.3")