JAMES-4098 ReadOnlyUsersLDAPRepository::isAdministrator should take i…

…nto account the multiple administrators if configured

Default to the current LDAP configuration `administrator` entry, if empty fallback to the multiple administrators one.

server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java

@@ -33,6 +33,7 @@
 import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.user.lib.UsersRepositoryImpl;
 
+import com.github.fge.lambdas.Throwing;
 import com.unboundid.ldap.sdk.LDAPConnectionPool;
 import com.unboundid.ldap.sdk.LDAPException;
 
@@ -211,14 +212,28 @@ public boolean supportVirtualHosting() {
         return ldapConfiguration.supportsVirtualHosting();
     }
 
+    /**
+     * Determines if the given username has administrator privileges.
+     * <p>
+     * If the {@code administratorId} is set in the LDAP configuration, the method will return
+     * {@code true} only if the given username matches the configured administrator ID.
+     * <p>
+     * If the {@code administratorId} is not set, the method falls back to the default
+     * administrator determination provided by the parent implementation which could
+     * support a list of administrators.
+     * </p>
+     *
+     * @param username The {@link Username} to check for administrator privileges.
+     * @return {@code true} if the username has administrator privileges, {@code false} otherwise.
+     * @throws UsersRepositoryException If an error occurs while validating the username.
+     */
     @Override
     public boolean isAdministrator(Username username) throws UsersRepositoryException {
         assertValid(username);
 
-        if (ldapConfiguration.getAdministratorId().isPresent()) {
-            return ldapConfiguration.getAdministratorId().get().equals(username);
-        }
-        return false;
+        return ldapConfiguration.getAdministratorId()
+            .map(ldapAdministratorAttribute -> ldapAdministratorAttribute.equals(username))
+            .orElseGet(Throwing.supplier(() -> super.isAdministrator(username)));
     }
 
     @Override

server/data/data-ldap/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java

@@ -29,6 +29,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.util.Optional;
+import java.util.Set;
 
 import org.apache.commons.configuration2.HierarchicalConfiguration;
 import org.apache.commons.configuration2.ex.ConversionException;
@@ -259,11 +260,31 @@ public UsersRepository testee(Optional<Username> administrator) throws Exception
             return startUsersRepository(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer, administrator), testSystem.getDomainList());
         }
 
+        @Override
+        public UsersRepository testee(Set<Username> administrators) throws Exception {
+            return startUsersRepository(ldapRepositoryConfigurationWithVirtualHosting(ldapContainer, administrators), testSystem.getDomainList());
+        }
+
         @Test
         void isAdministratorShouldReturnTrueWhenConfiguredAndUserIsAdmin(TestSystem testSystem) throws Exception {
             assertThat(testee().isAdministrator(testSystem.getAdmin())).isTrue();
         }
 
+        @Test
+        void isAdministratorShouldReturnTrueWhenConfiguredMultipleAdminsAndUserIsAdmin(TestSystem testSystem) throws Exception {
+            UsersRepository testee = testee(Set.of(testSystem.getAdmin(), testSystem.getUser1()));
+
+            assertThat(testee.isAdministrator(testSystem.getAdmin())).isTrue();
+            assertThat(testee.isAdministrator(testSystem.getUser1())).isTrue();
+        }
+
+        @Test
+        void isAdministratorShouldReturnFalseWhenConfiguredAndUserIsNotAdmin(TestSystem testSystem) throws Exception {
+            UsersRepository testee = testee(Set.of(testSystem.getAdmin(), testSystem.getUser1()));
+
+            assertThat(testee.isAdministrator(testSystem.getUser2())).isFalse();
+        }
+
         @Test
         void knownUserShouldBeAbleToLogInWhenPasswordIsCorrectWithVirtualHosting() throws Exception {
             assertThat(usersRepository.test(JAMES_USER_MAIL, PASSWORD)).isEqualTo(Optional.of(JAMES_USER_MAIL));
@@ -365,6 +386,11 @@ public UsersRepository testee(Optional<Username> administrator) throws Exception
             return startUsersRepository(ldapRepositoryConfiguration(ldapContainer, administrator), testSystem.getDomainList());
         }
 
+        @Override
+        public UsersRepository testee(Set<Username> administrators) throws Exception {
+            return startUsersRepository(ldapRepositoryConfiguration(ldapContainer, administrators), testSystem.getDomainList());
+        }
+
         @Test
         void knownUserShouldBeAbleToLogInWhenPasswordIsCorrect() throws Exception {
             assertThat(usersRepository.test(JAMES_USER, PASSWORD)).isEqualTo(Optional.of(JAMES_USER));
@@ -415,6 +441,21 @@ void isAdministratorShouldReturnTrueWhenConfiguredAndUserIsAdmin(TestSystem test
             assertThat(testee().isAdministrator(testSystem.getAdmin())).isTrue();
         }
 
+        @Test
+        void isAdministratorShouldReturnTrueWhenConfiguredMultipleAdminsAndUserIsAdmin(TestSystem testSystem) throws Exception {
+            UsersRepository testee = testee(Set.of(testSystem.getAdmin(), testSystem.getUser1()));
+
+            assertThat(testee.isAdministrator(testSystem.getAdmin())).isTrue();
+            assertThat(testee.isAdministrator(testSystem.getUser1())).isTrue();
+        }
+
+        @Test
+        void isAdministratorShouldReturnFalseWhenConfiguredAndUserIsNotAdmin(TestSystem testSystem) throws Exception {
+            UsersRepository testee = testee(Set.of(testSystem.getAdmin(), testSystem.getUser1()));
+
+            assertThat(testee.isAdministrator(testSystem.getUser2())).isFalse();
+        }
+
         @Disabled("JAMES-3088 Users are provisioned by default from Dockerfile, cannot setup this test case," +
             "See @link{ReadOnlyUsersLDAPRepositoryEmptyListTest}")
         @Override
@@ -524,6 +565,13 @@ static HierarchicalConfiguration<ImmutableNode> ldapRepositoryConfiguration(Ldap
         return configuration;
     }
 
+    static HierarchicalConfiguration<ImmutableNode> ldapRepositoryConfiguration(LdapGenericContainer ldapContainer, Set<Username> administrators) {
+        PropertyListConfiguration configuration = baseConfiguration(ldapContainer);
+        configuration.addProperty("[@userIdAttribute]", "uid");
+        administrators.forEach(admin -> configuration.addProperty("administratorIds.administratorId", admin.asString()));
+        return configuration;
+    }
+
     static HierarchicalConfiguration<ImmutableNode> ldapRepositoryConfigurationWithVirtualHosting(LdapGenericContainer ldapContainer) {
         return ldapRepositoryConfigurationWithVirtualHosting(ldapContainer, Optional.of(ADMIN));
     }
@@ -536,6 +584,14 @@ static HierarchicalConfiguration<ImmutableNode> ldapRepositoryConfigurationWithV
         return configuration;
     }
 
+    static HierarchicalConfiguration<ImmutableNode> ldapRepositoryConfigurationWithVirtualHosting(LdapGenericContainer ldapContainer, Set<Username> administrators) {
+        PropertyListConfiguration configuration = baseConfiguration(ldapContainer);
+        configuration.addProperty("[@userIdAttribute]", "mail");
+        configuration.addProperty("supportsVirtualHosting", true);
+        administrators.forEach(admin -> configuration.addProperty("administratorIds.administratorId", admin.asString()));
+        return configuration;
+    }
+
     private static PropertyListConfiguration baseConfiguration(LdapGenericContainer ldapContainer) {
         PropertyListConfiguration configuration = new PropertyListConfiguration();
         configuration.addProperty("[@ldapHost]", ldapContainer.getLdapHost());

server/data/data-library/src/test/java/org/apache/james/user/lib/UsersRepositoryContract.java

@@ -141,6 +141,14 @@ public Username getAdmin() {
             return admin;
         }
 
+        public Username getUser1() {
+            return user1;
+        }
+
+        public Username getUser2() {
+            return user2;
+        }
+
         public Username getUserWithUnknownDomain() {
             return userWithUnknownDomain;
         }