Returning tokenGroups attribute as SID string instead of byte array (#63509)

* Returning tokenGroups attribute as SID string instead of byte array (AD metadata)

Resolves: #61173

* Returning tokenGroups attribute as SID string instead of byte array (AD metadata)

Resolves: #61173

* Adding test

* Adding test

* Adding test

* Fixing test

* Fixing test

* Addressing PR comments

* Nit fix

* Nit fixes

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryGroupsResolver.java

@@ -27,6 +27,7 @@ import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.core.security.authc.ldap.support.SessionFactorySettings.IGNORE_REFERRAL_ERRORS_SETTING;
 import static org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySIDUtil.convertToString;
+import static org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySIDUtil.TOKEN_GROUPS;
 import static org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySessionFactory.buildDnFromDomain;
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.OBJECT_CLASS_PRESENCE_FILTER;
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.search;
@@ -34,7 +35,6 @@ import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.sear
 
 class ActiveDirectoryGroupsResolver implements GroupsResolver {
 
-    private static final String TOKEN_GROUPS = "tokenGroups";
     private final String baseDn;
     private final LdapSearchScope scope;
     private final boolean ignoreReferralErrors;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySIDUtil.java

@@ -26,9 +26,9 @@ package org.elasticsearch.xpack.security.authc.ldap;
 
 import org.apache.commons.codec.binary.Hex;
 
-class ActiveDirectorySIDUtil {
-
-    static String convertToString( byte[] bytes )
+public class ActiveDirectorySIDUtil {
+    public static final String TOKEN_GROUPS = "tokenGroups";
+    public static String convertToString(byte[] bytes)
     {
         /*
          * The binary data structure, from http://msdn.microsoft.com/en-us/library/cc230371(PROT.10).aspx:

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapMetadataResolver.java

@@ -23,6 +23,8 @@ import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySIDUtil.convertToString;
+import static org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySIDUtil.TOKEN_GROUPS;
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.OBJECT_CLASS_PRESENCE_FILTER;
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.searchForEntry;
 
@@ -76,8 +78,14 @@ public class LdapMetadataResolver {
                                 attr -> attr.getName(),
                                 attr -> {
                                     final String[] values = attr.getValues();
+                                    if(attr.getName().equals(TOKEN_GROUPS)) {
+                                        return values.length == 1 ? convertToString(attr.getValueByteArrays()[0]) :
+                                            Arrays.stream(attr.getValueByteArrays())
+                                            .map((sidBytes) -> convertToString(sidBytes))
+                                            .collect(Collectors.toList());
+                                    }
                                     return values.length == 1 ? values[0] : List.of(values);
-                                })
+                                 })
                         );
     }
 

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryGroupsResolverTests.java

@@ -17,6 +17,7 @@ import org.junit.Before;
 import java.util.List;
 import java.util.regex.Pattern;
 
+
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasItem;

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java

@@ -10,6 +10,7 @@ import com.unboundid.ldap.sdk.ResultCode;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.concurrent.UncategorizedExecutionException;
 import org.elasticsearch.env.Environment;
@@ -20,11 +21,13 @@ import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 import org.elasticsearch.xpack.core.security.authc.ldap.ActiveDirectorySessionFactorySettings;
 import org.elasticsearch.xpack.core.security.authc.ldap.LdapRealmSettings;
+import org.elasticsearch.xpack.core.security.authc.ldap.support.LdapMetadataResolverSettings;
 import org.elasticsearch.xpack.core.security.authc.ldap.support.LdapSearchScope;
 import org.elasticsearch.xpack.core.security.authc.ldap.support.SessionFactorySettings;
 import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings;
 import org.elasticsearch.xpack.core.ssl.SSLService;
 import org.elasticsearch.xpack.core.ssl.VerificationMode;
+import org.elasticsearch.xpack.security.authc.ldap.support.LdapMetadataResolver;
 import org.elasticsearch.xpack.security.authc.ldap.support.LdapSession;
 import org.elasticsearch.xpack.security.authc.ldap.support.LdapTestCase;
 import org.elasticsearch.xpack.security.authc.ldap.support.SessionFactory;
@@ -33,14 +36,18 @@ import org.junit.Before;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 
 import static org.elasticsearch.xpack.core.security.authc.RealmSettings.getFullSettingKey;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.everyItem;
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.matchesPattern;
 
 public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryTestCase {
 
@@ -48,6 +55,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryT
     private static final RealmConfig.RealmIdentifier REALM_ID = new RealmConfig.RealmIdentifier("active_directory", REALM_NAME);
     private final SecureString SECURED_PASSWORD = new SecureString(PASSWORD);
     private ThreadPool threadPool;
+    private static final String BRUCE_BANNER_DN = "cn=Bruce Banner,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com";
 
     @Before
     public void init() throws Exception {
@@ -367,6 +375,35 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryT
         }
     }
 
+    @SuppressWarnings("unchecked")
+    public void testResolveTokenGroupsSID() throws Exception {
+        Settings settings = Settings.builder()
+            .put("path.home", createTempDir())
+            .put(RealmSettings.getFullSettingKey(REALM_ID, RealmSettings.ORDER_SETTING), 0)
+            .put(buildAdSettings(REALM_ID, AD_LDAP_URL, AD_DOMAIN, "CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com",
+                LdapSearchScope.SUB_TREE, false))
+            .put(ActiveDirectorySessionFactorySettings.AD_GROUP_SEARCH_BASEDN_SETTING, "DC=ad,DC=test,DC=elasticsearch,DC=com")
+            .put(ActiveDirectorySessionFactorySettings.AD_GROUP_SEARCH_SCOPE_SETTING, LdapSearchScope.SUB_TREE)
+            .put(getFullSettingKey(REALM_ID, LdapMetadataResolverSettings.ADDITIONAL_METADATA_SETTING), "tokenGroups")
+            .build();
+        RealmConfig config = configureRealm("ad-test", LdapRealmSettings.AD_TYPE, settings);
+        final PlainActionFuture<Map<String, Object>> future = new PlainActionFuture<>();
+        LdapMetadataResolver resolver = new LdapMetadataResolver(config, true);
+        try (ActiveDirectorySessionFactory sessionFactory = getActiveDirectorySessionFactory(config, sslService, threadPool)) {
+            String userName = "hulk";
+            try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
+                assertConnectionCanReconnect(ldap.getConnection());
+                resolver.resolve(ldap.getConnection(), BRUCE_BANNER_DN, TimeValue.timeValueSeconds(1), logger, null, future);
+                Map<String, Object> metadataGroupSIDs = future.get();
+                assertThat(metadataGroupSIDs.size(), equalTo(1));
+                assertNotNull(metadataGroupSIDs.get("tokenGroups"));
+                List<String> SIDs = ((List<String>) metadataGroupSIDs.get("tokenGroups"));
+                assertThat(SIDs.size(), equalTo(7));
+                assertThat(SIDs, everyItem(matchesPattern("S-1-5-(?:21|32)-\\d+(?:-\\d+\\-\\d+\\-\\d+)?")));
+            }
+        }
+    }
+
     private Settings buildAdSettings(String ldapUrl, String adDomainName, boolean hostnameVerification) {
         return buildAdSettings(ldapUrl, adDomainName, hostnameVerification, randomBoolean());
     }