Add support for splitting saml groups by delimiter (#102769)

jfreden · web-flow · commit 077b47db3703 · 2023-12-07T08:55:46.000+01:00
* Add support for splitting saml groups by delimiter
diff --git 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}::
+(<<static-cluster-setting,Static>>)
+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}::
 (<<static-cluster-setting,Static>>)
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
@@ -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<String> 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:
+     * <ul>
+     * <li>The name of the SAML attribute to use</li>
+     * <li>A delimiter to apply to that attribute value in order to extract the substrings that should be used.</li>
+     * </ul>
+     * 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<String> 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<String> getDelimiter() {
+            return this.delimiter;
+        }
+
+        public Collection<Setting.AffixSetting<?>> settings() {
+            List<Setting.AffixSetting<?>> 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
@@ -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<String> 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
@@ -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<User> result = performAuthentication(
-            roleMapper,
-            useNameId,
-            principalIsEmailAddress,
-            populateUserMetadata,
-            false,
-            authenticatingRealm
-        );
+        final boolean testWithDelimiter = randomBoolean();
+        final AuthenticationResult<User> 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<User> performAuthentication(
         Boolean populateUserMetadata,
         boolean useAuthorizingRealm,
         String authenticatingRealm
+    ) throws Exception {
+        return performAuthentication(
+            roleMapper,
+            useNameId,
+            principalIsEmailAddress,
+            populateUserMetadata,
+            useAuthorizingRealm,
+            authenticatingRealm,
+            Arrays.asList("avengers", "shield"),
+            null
+        );
+    }
+
+    private AuthenticationResult<User> performAuthentication(
+        UserRoleMapper roleMapper,
+        boolean useNameId,
+        boolean principalIsEmailAddress,
+        Boolean populateUserMetadata,
+        boolean useAuthorizingRealm,
+        String authenticatingRealm,
+        List<String> groups,
+        String groupsDelimiter
     ) throws Exception {
         final EntityDescriptor idp = mockIdp();
         final SpConfiguration sp = new SpConfiguration("<sp>", "https://saml/", null, null, null, Collections.emptyList());
@@ -453,8 +495,12 @@ private AuthenticationResult<User> 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<User> 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<String> strings = performAttributeSelectionWithSplit(",", "departments", "engineering", "elasticsearch-admins", "employees");
+        assertThat("For attributes: " + strings, strings, contains("engineering", "elasticsearch-admins", "employees"));
+    }
+
+    public void testAttributeSelectionWithSplitEmptyInput() {
+        List<String> strings = performAttributeSelectionWithSplit(",", "departments");
+        assertThat("For attributes: " + strings, strings, is(empty()));
+    }
+
+    public void testAttributeSelectionWithSplitJustDelimiter() {
+        List<String> strings = performAttributeSelectionWithSplit(",", ",");
+        assertThat("For attributes: " + strings, strings, is(empty()));
+    }
+
+    public void testAttributeSelectionWithSplitNoDelimiter() {
+        List<String> strings = performAttributeSelectionWithSplit(",", "departments", "elasticsearch-team");
+        assertThat("For attributes: " + strings, strings, contains("elasticsearch-team"));
+    }
+
+    private List<String> 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")
