Resolve user attribute names once

jfreden · Nov 27, 2024 · acd4205 · acd4205
1 parent 4a3de6a
commit acd4205
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 12 deletions.
diff --git a/...ore/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java b/...ore/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java
@@ -231,6 +231,17 @@ public static Set<Setting.AffixSetting<?>> getSettings(String type) {
         return set;
     }
 
+    public record UserAttributeNameConfiguration(String principal, String dn, String name, String mail) {
+        public static UserAttributeNameConfiguration fromConfig(RealmConfig config) {
+            return new UserAttributeNameConfiguration(
+                PRINCIPAL_ATTRIBUTE.apply(config.type()).name(config),
+                DN_ATTRIBUTE.apply(config.type()).name(config),
+                NAME_ATTRIBUTE.apply(config.type()).name(config),
+                MAIL_ATTRIBUTE.apply(config.type()).name(config)
+            );
+        }
+    }
+
     /**
      * The SAML realm offers a number of settings that rely on attributes that are populate by the Identity Provider in the SAML Response.
      * Each attribute has 2 settings:

diff --git a/.../plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java b/.../plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java
@@ -171,6 +171,7 @@ public final class SamlRealm extends Realm implements Releasable {
     private final Supplier<EntityDescriptor> idpDescriptor;
 
     private final SpConfiguration serviceProvider;
+    private final SamlRealmSettings.UserAttributeNameConfiguration userAttributeNameConfiguration;
     private final SamlAuthnRequestBuilder.NameIDPolicySettings nameIdPolicy;
     private final Boolean forceAuthn;
     private final boolean useSingleLogout;
@@ -224,6 +225,8 @@ public static SamlRealm create(
             serviceProvider,
             maxSkew
         );
+        SamlRealmSettings.UserAttributeNameConfiguration userAttributeNameConfiguration = SamlRealmSettings.UserAttributeNameConfiguration
+            .fromConfig(config);
 
         final SamlRealm realm = new SamlRealm(
             config,
@@ -232,7 +235,8 @@ public static SamlRealm create(
             logoutHandler,
             logoutResponseHandler,
             idpDescriptor,
-            serviceProvider
+            serviceProvider,
+            userAttributeNameConfiguration
         );
 
         // the metadata resolver needs to be destroyed since it runs a timer task in the background and destroying stops it!
@@ -253,7 +257,8 @@ public SpConfiguration getServiceProvider() {
         SamlLogoutRequestHandler logoutHandler,
         SamlLogoutResponseHandler logoutResponseHandler,
         Supplier<EntityDescriptor> idpDescriptor,
-        SpConfiguration spConfiguration
+        SpConfiguration spConfiguration,
+        SamlRealmSettings.UserAttributeNameConfiguration userAttributeNameConfiguration
     ) throws Exception {
         super(config);
 
@@ -268,6 +273,7 @@ public SpConfiguration getServiceProvider() {
 
         this.idpDescriptor = idpDescriptor;
         this.serviceProvider = spConfiguration;
+        this.userAttributeNameConfiguration = userAttributeNameConfiguration;
 
         this.nameIdPolicy = new SamlAuthnRequestBuilder.NameIDPolicySettings(
             config.getSetting(NAMEID_FORMAT),
@@ -556,11 +562,7 @@ public void authenticate(AuthenticationToken authenticationToken, ActionListener
     }
 
     private void buildUser(SamlAttributes attributes, ActionListener<AuthenticationResult<User>> baseListener) {
-        final String principal = resolveSingleValueAttribute(
-            attributes,
-            principalAttribute,
-            PRINCIPAL_ATTRIBUTE.apply(config.type()).name(config)
-        );
+        final String principal = resolveSingleValueAttribute(attributes, principalAttribute, userAttributeNameConfiguration.principal());
         if (Strings.isNullOrEmpty(principal)) {
             final String msg = principalAttribute
                 + " not found in saml attributes"
@@ -606,9 +608,9 @@ private void buildUser(SamlAttributes attributes, ActionListener<AuthenticationR
         final Map<String, Object> userMeta = Map.copyOf(userMetaBuilder);
 
         final List<String> groups = groupsAttribute.getAttribute(attributes);
-        final String dn = resolveSingleValueAttribute(attributes, dnAttribute, DN_ATTRIBUTE.apply(config.type()).name(config));
-        final String name = resolveSingleValueAttribute(attributes, nameAttribute, NAME_ATTRIBUTE.apply(config.type()).name(config));
-        final String mail = resolveSingleValueAttribute(attributes, mailAttribute, MAIL_ATTRIBUTE.apply(config.type()).name(config));
+        final String dn = resolveSingleValueAttribute(attributes, dnAttribute, userAttributeNameConfiguration.dn());
+        final String name = resolveSingleValueAttribute(attributes, nameAttribute, userAttributeNameConfiguration.name());
+        final String mail = resolveSingleValueAttribute(attributes, mailAttribute, userAttributeNameConfiguration.mail());
         UserRoleMapper.UserData userData = new UserRoleMapper.UserData(principal, dn, groups, userMeta, config);
         logger.debug("SAML attribute mapping = [{}]", userData);
         roleMapper.resolveRoles(userData, wrappedListener.delegateFailureAndWrap((l, roles) -> {

diff --git a/...curity/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java b/...curity/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTestHelper.java
@@ -8,6 +8,7 @@
 
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
+import org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings;
 import org.elasticsearch.xpack.core.security.authc.support.UserRoleMapper;
 import org.opensaml.saml.common.xml.SAMLConstants;
 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
@@ -56,7 +57,8 @@ public static SamlRealm buildRealm(RealmConfig realmConfig, @Nullable X509Creden
             mock(SamlLogoutRequestHandler.class),
             mock(SamlLogoutResponseHandler.class),
             () -> idpDescriptor,
-            spConfiguration
+            spConfiguration,
+            mock(SamlRealmSettings.UserAttributeNameConfiguration.class)
         );
     }
 

diff --git a/...in/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java b/...in/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java
@@ -648,7 +648,16 @@ public SamlRealm buildRealm(
         SpConfiguration sp
     ) throws Exception {
         try {
-            return new SamlRealm(config, roleMapper, authenticator, logoutHandler, mock(SamlLogoutResponseHandler.class), () -> idp, sp);
+            return new SamlRealm(
+                config,
+                roleMapper,
+                authenticator,
+                logoutHandler,
+                mock(SamlLogoutResponseHandler.class),
+                () -> idp,
+                sp,
+                mock(SamlRealmSettings.UserAttributeNameConfiguration.class)
+            );
         } catch (SettingsException e) {
             logger.info(() -> format("Settings are invalid:\n%s", config.settings().toDelimitedString('\n')), e);
             throw e;