Merge pull request #735 from Netcentric/feature/732-validation-to-ens…

…ure-groups-with-externalId-are-not-used-in-isMemberOf

Validation to ensure groups with externalId set are not used in isMemberOf

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/configmodel/GlobalConfiguration.java

@@ -29,6 +29,7 @@ public class GlobalConfiguration {
 
     public static final String KEY_ALLOW_CREATE_OF_UNMANAGED_RELATIONSHIPS = "allowCreateOfUnmanagedRelationships";
 
+    public static final String KEY_ALLOW_EXTERNAL_GROUPS_IN_IS_MEMBER_OF = "allowExternalGroupsInIsMemberOf";
 
     public static final String KEY_AUTOCREATE_TEST_USERS = "autoCreateTestUsers";
 
@@ -42,6 +43,8 @@ public class GlobalConfiguration {
     private Pattern defaultUnmanagedExternalMembersRegex;
     private String defaultUnmanagedAcePathsRegex;
     private Boolean allowCreateOfUnmanagedRelationships = null;
+
+    private Boolean allowExternalGroupsInIsMemberOf = null;
 
     private AutoCreateTestUsersConfig autoCreateTestUsersConfig;
 
@@ -88,6 +91,11 @@ public GlobalConfiguration(Map<String, ?> globalConfigMap) {
             if (globalConfigMap.containsKey(KEY_AUTOCREATE_TEST_USERS)) {
                 autoCreateTestUsersConfig = new AutoCreateTestUsersConfig((Map) globalConfigMap.get(KEY_AUTOCREATE_TEST_USERS));
             }
+
+            if (globalConfigMap.containsKey(KEY_ALLOW_EXTERNAL_GROUPS_IN_IS_MEMBER_OF)) {
+                setAllowExternalGroupsInIsMemberOf(Boolean.valueOf(globalConfigMap.get(KEY_ALLOW_EXTERNAL_GROUPS_IN_IS_MEMBER_OF).toString()));
+            }
+
         }
 
     }
@@ -147,6 +155,14 @@ public void merge(GlobalConfiguration otherGlobalConfig) {
             }
         }
 
+        if (otherGlobalConfig.getAllowExternalGroupsInIsMemberOf() != null) {
+            if (allowExternalGroupsInIsMemberOf == null) {
+                allowExternalGroupsInIsMemberOf = otherGlobalConfig.getAllowExternalGroupsInIsMemberOf();
+            } else {
+                throw new IllegalArgumentException("Duplicate config for " + KEY_ALLOW_EXTERNAL_GROUPS_IN_IS_MEMBER_OF);
+            }
+        }
+
     }
 
     public String getMinRequiredVersion() {
@@ -205,7 +221,13 @@ static Pattern stringToRegex(String regex) {
     public AutoCreateTestUsersConfig getAutoCreateTestUsersConfig() {
         return autoCreateTestUsersConfig;
     }
-
-
 
+    public Boolean getAllowExternalGroupsInIsMemberOf() {
+        return allowExternalGroupsInIsMemberOf;
+    }
+
+    public void setAllowExternalGroupsInIsMemberOf(Boolean allowExternalGroupsInIsMemberOf) {
+        this.allowExternalGroupsInIsMemberOf = allowExternalGroupsInIsMemberOf;
+    }
+
 }

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/configreader/YamlConfigurationMerger.java

@@ -45,6 +45,7 @@
 import biz.netcentric.cq.tools.actool.history.impl.PersistableInstallationLogger;
 import biz.netcentric.cq.tools.actool.slingsettings.ExtendedSlingSettingsService;
 import biz.netcentric.cq.tools.actool.validators.AuthorizableValidator;
+import biz.netcentric.cq.tools.actool.validators.ExternalGroupsInIsMemberOfValidator;
 import biz.netcentric.cq.tools.actool.validators.ConfigurationsValidator;
 import biz.netcentric.cq.tools.actool.validators.GlobalConfigurationValidator;
 import biz.netcentric.cq.tools.actool.validators.ObsoleteAuthorizablesValidator;
@@ -68,6 +69,9 @@ public class YamlConfigurationMerger implements ConfigurationMerger {
     @Reference(policyOption = ReferencePolicyOption.GREEDY)
     ObsoleteAuthorizablesValidator obsoleteAuthorizablesValidator;
 
+    @Reference(policyOption = ReferencePolicyOption.GREEDY)
+    ExternalGroupsInIsMemberOfValidator externalGroupsInIsMemberOfValidator;
+
     @Reference(policyOption = ReferencePolicyOption.GREEDY)
     VirtualGroupProcessor virtualGroupProcessor;
 
@@ -222,7 +226,9 @@ public AcConfiguration getMergedConfigurations(
         if(!Boolean.TRUE.equals(globalConfiguration.getAllowCreateOfUnmanagedRelationships())) {
             UnmangedExternalMemberRelationshipChecker.validate(acConfiguration);
         }
-
+
+        externalGroupsInIsMemberOfValidator.validateIsMemberOfConfig(acConfiguration, installLog, globalConfiguration);
+
         installLog.setMergedAndProcessedConfig(
                 "# Merged configuration of " + configFileContentByFilename.size() + " files \n" + acConfiguration);
 
@@ -231,6 +237,7 @@ public AcConfiguration getMergedConfigurations(
         return acConfiguration;
     }
 
+
     private Map<String, Object> getGlobalVariablesForYamlMacroProcessing() {
         Map<String, Object> globalVariables = new HashMap<>();
         if(slingSettingsService != null) {

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/validators/ExternalGroupsInIsMemberOfValidator.java

@@ -0,0 +1,77 @@
+/*
+ * (C) Copyright 2024 Netcentric AG.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package biz.netcentric.cq.tools.actool.validators;
+
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import org.apache.commons.lang3.StringUtils;
+import org.osgi.service.component.annotations.Component;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import biz.netcentric.cq.tools.actool.configmodel.AcConfiguration;
+import biz.netcentric.cq.tools.actool.configmodel.AuthorizableConfigBean;
+import biz.netcentric.cq.tools.actool.configmodel.AuthorizablesConfig;
+import biz.netcentric.cq.tools.actool.configmodel.GlobalConfiguration;
+import biz.netcentric.cq.tools.actool.history.impl.PersistableInstallationLogger;
+import biz.netcentric.cq.tools.actool.validators.exceptions.InvalidExternalGroupUsageValidationException;
+
+@Component(service = ExternalGroupsInIsMemberOfValidator.class)
+public class ExternalGroupsInIsMemberOfValidator {
+    private static final Logger LOG = LoggerFactory.getLogger(ExternalGroupsInIsMemberOfValidator.class);
+
+    public void validateIsMemberOfConfig(AcConfiguration acConfiguration, PersistableInstallationLogger installLog,
+            GlobalConfiguration globalConfiguration) throws InvalidExternalGroupUsageValidationException {
+        List<String> externalGroupsValidationResults = checkIsMemberOfConfigsOfAllAuthorizables(acConfiguration);
+        if (!externalGroupsValidationResults.isEmpty()) {
+
+            externalGroupsValidationResults.stream().forEach(m -> installLog.addWarning(LOG, m));
+
+            String validationMsg = "Found " + externalGroupsValidationResults.size() + " authorizable(s) that use external groups in isMemberOf. ";
+
+            if (Boolean.TRUE.equals(globalConfiguration.getAllowExternalGroupsInIsMemberOf())) {
+                installLog.addWarning(LOG, validationMsg);
+                installLog.addWarning(LOG, "Found global config 'allowExternalGroupsInIsMemberOf: true': PLEASE REFACTOR your groups structure to not use external groups in isMemberOf.");
+            } else {
+                installLog.addError(LOG, validationMsg + " If absolutely needed, use 'allowExternalGroupsInIsMemberOf: true' in global configuration, but prefer to refactor your groups structure to not use isMemberOf together with external groups.", null);
+                throw new InvalidExternalGroupUsageValidationException(validationMsg);
+            }
+        }
+    }
+
+    private List<String> checkIsMemberOfConfigsOfAllAuthorizables(AcConfiguration acConfiguration) {
+
+        List<String> validationErrors = new LinkedList<>();
+
+        AuthorizablesConfig authorizablesConfig = acConfiguration.getAuthorizablesConfig();
+        for (AuthorizableConfigBean authorizableConfigBean : authorizablesConfig) {
+            if(authorizableConfigBean.getIsMemberOf() == null) {
+                continue;
+            }
+
+            List<String> groupIdsWithExternalIdSet = Arrays.stream(authorizableConfigBean.getIsMemberOf())
+                    .map(aId -> acConfiguration.getAuthorizablesConfig().getAuthorizableConfig(aId))
+                    .filter(Objects::nonNull)
+                    .filter(authBean -> StringUtils.isNotBlank(authBean.getExternalId()))
+                    .map(AuthorizableConfigBean::getAuthorizableId)
+                    .collect(Collectors.toList());
+
+            if (!groupIdsWithExternalIdSet.isEmpty()) {
+                validationErrors.add("The authorizable " + authorizableConfigBean.getAuthorizableId()
+                        + " cannot use external group(s) in isMemberOf list: " + StringUtils.join(groupIdsWithExternalIdSet, ","));
+            }
+        }
+
+        return validationErrors;
+    }
+}

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/validators/exceptions/InvalidExternalGroupUsageValidationException.java

@@ -0,0 +1,19 @@
+/*
+ * (C) Copyright 2015 Netcentric AG.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package biz.netcentric.cq.tools.actool.validators.exceptions;
+
+public class InvalidExternalGroupUsageValidationException extends AcConfigBeanValidationException {
+    public InvalidExternalGroupUsageValidationException(String message) {
+        super(message);
+    }
+
+    public InvalidExternalGroupUsageValidationException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/configreader/YamlConfigurationMergerTest.java

@@ -12,8 +12,8 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
 import static org.mockito.MockitoAnnotations.initMocks;
@@ -38,6 +38,7 @@
 import biz.netcentric.cq.tools.actool.configmodel.AuthorizableConfigBean;
 import biz.netcentric.cq.tools.actool.configmodel.AuthorizablesConfig;
 import biz.netcentric.cq.tools.actool.history.impl.PersistableInstallationLogger;
+import biz.netcentric.cq.tools.actool.validators.ExternalGroupsInIsMemberOfValidator;
 import biz.netcentric.cq.tools.actool.validators.exceptions.AcConfigBeanValidationException;
 import biz.netcentric.cq.tools.actool.validators.impl.ObsoleteAuthorizablesValidatorImpl;
 
@@ -151,6 +152,7 @@ public static YamlConfigurationMerger getConfigurationMerger() {
         merger.obsoleteAuthorizablesValidator = new ObsoleteAuthorizablesValidatorImpl();
         merger.virtualGroupProcessor = new VirtualGroupProcessor();
         merger.testUserConfigsCreator = new TestUserConfigsCreator();
+        merger.externalGroupsInIsMemberOfValidator = new ExternalGroupsInIsMemberOfValidator();
         return merger;
     }
 }

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/validators/ExternalGroupsInIsMemberOfValidatorTest.java

@@ -0,0 +1,44 @@
+package biz.netcentric.cq.tools.actool.validators;
+
+import static biz.netcentric.cq.tools.actool.configreader.YamlConfigurationMergerTest.getAcConfigurationForFile;
+import static biz.netcentric.cq.tools.actool.configreader.YamlConfigurationMergerTest.getConfigurationMerger;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import javax.jcr.Session;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+
+import biz.netcentric.cq.tools.actool.configmodel.AcConfiguration;
+import biz.netcentric.cq.tools.actool.validators.exceptions.InvalidExternalGroupUsageValidationException;
+
+class ExternalGroupsInIsMemberOfValidatorTest {
+
+    @Mock
+    Session session;
+
+    @Test
+    public void testExternalIdSetCorrectlyOnRoleOnly() throws Exception {
+        AcConfiguration acConfigurationForFile = getAcConfigurationForFile(getConfigurationMerger(), session,
+                "externalIds/test-externalId-set-correctly-on-role-only.yaml");
+
+        assertEquals(3, acConfigurationForFile.getAuthorizablesConfig().size());
+    }
+
+    @Test
+    public void testExternalIdSetOnFragmentOverruledByConfigToBeAlllowed() throws Exception {
+        AcConfiguration acConfigurationForFile = getAcConfigurationForFile(getConfigurationMerger(), session,
+                "externalIds/test-externalId-set-on-fragment-overruled-by-config-to-be-allowed.yaml");
+
+        assertEquals(3, acConfigurationForFile.getAuthorizablesConfig().size());
+    }
+
+    @Test
+    public void testExternalIdSetOnFragmentInvalid() {
+        assertThrows(InvalidExternalGroupUsageValidationException.class,
+                () -> getAcConfigurationForFile(getConfigurationMerger(), session,
+                        "externalIds/test-externalId-set-on-fragment-invalid.yaml"));
+    }
+
+}

accesscontroltool-bundle/src/test/resources/externalIds/test-externalId-set-correctly-on-role-only.yaml

@@ -0,0 +1,27 @@
+#
+# (C) Copyright 2024 Netcentric AG.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+
+- global_config:
+
+      # default - no extra config set
+      # allowExternalGroupsInIsMemberOf: true
+
+- group_config:
+
+    - fragment-1:
+       - name: "Fragment 1"
+
+    - fragment-2:
+       - name: "Fragment 2"
+
+    - role-1:
+       - name: "Role 1"
+         isMemberOf: fragment-1, fragment-2
+         externalId: role-1;ims
+

accesscontroltool-bundle/src/test/resources/externalIds/test-externalId-set-on-fragment-invalid.yaml

@@ -0,0 +1,28 @@
+#
+# (C) Copyright 2024 Netcentric AG.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+
+- global_config:
+
+      # default - no extra config set
+      # allowExternalGroupsInIsMemberOf: true
+
+- group_config:
+
+    - fragment-1:
+       - name: "Fragment 1"
+
+    - fragment-2:
+       - name: "Fragment 2"
+         externalId: fragment-2;ims
+
+    - role-1:
+       - name: "Role 1"
+         isMemberOf: fragment-1, fragment-2
+
+

accesscontroltool-bundle/src/test/resources/externalIds/test-externalId-set-on-fragment-overruled-by-config-to-be-allowed.yaml

@@ -0,0 +1,27 @@
+#
+# (C) Copyright 2024 Netcentric AG.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+
+- global_config:
+
+      allowExternalGroupsInIsMemberOf: true
+
+- group_config:
+
+    - fragment-1:
+       - name: "Fragment 1"
+
+    - fragment-2:
+       - name: "Fragment 2"
+      externalId: fragment-2;ims
+
+    - role-1:
+       - name: "Role 1"
+         isMemberOf: fragment-1, fragment-2
+
+

docs/Configuration.md

@@ -52,7 +52,7 @@ property | comment | required
 --- | --- | ---
 name | Name of the group as shown in UI. Sets the property `profile/givenName` of that group. | optional
 description | Description of the group | optional
-externalId | Required for groups which are synchronized from [external sources](https://jackrabbit.apache.org/oak/docs/security/authentication/externalloginmodule.html) like [LDAP](https://jackrabbit.apache.org/oak/docs/security/authentication/ldap.html) or [Adobe IMS](https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/security/ims-support#aem-configuration). This establishes a connection between an (internal) JCR group and an externally managed group (and is persisted in the group's node in the property `rep:externalId`). The value has to be in format `<external-id>;<provider-name>`.  How the external ID and provider name look like is *External Identity Provider dependent*: For **Adobe IMS** it usually is `<groupId>;ims` while for **Oak LDAP** it usually is `<LDAP-DN>;<IDP-NAME>` where LDAP-DN is the full distinguished name and IDP-NAME is configured in OSGI config PID `org.apache.jackrabbit.oak.security.authentication.ldap.impl.LdapIdentityProvider` property `provider-name`. LDAP Example: `externalId: "cn=group-name,ou=mydepart,ou=Groups,dc=comp,dc=com;IDPNAME"`. Make sure to also set the group id according to how it is extracted by the external identify provider (configurable via OSGi configuration of the external identity provider). Since v1.9.3 | optional
+externalId | Required for groups which are synchronized from [external sources](https://jackrabbit.apache.org/oak/docs/security/authentication/externalloginmodule.html) like [LDAP](https://jackrabbit.apache.org/oak/docs/security/authentication/ldap.html) or [Adobe IMS](https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/security/ims-support#aem-configuration). This establishes a connection between an (internal) JCR group and an externally managed group (and is persisted in the group's node in the property `rep:externalId`). The value has to be in format `<external-id>;<provider-name>`.  How the external ID and provider name look like is *External Identity Provider dependent*: For **Adobe IMS** it usually is `<groupId>;ims` while for **Oak LDAP** it usually is `<LDAP-DN>;<IDP-NAME>` where LDAP-DN is the full distinguished name and IDP-NAME is configured in OSGI config PID `org.apache.jackrabbit.oak.security.authentication.ldap.impl.LdapIdentityProvider` property `provider-name`. LDAP Example: `externalId: "cn=group-name,ou=mydepart,ou=Groups,dc=comp,dc=com;IDPNAME"`. Make sure to also set the group id according to how it is extracted by the external identify provider (configurable via OSGi configuration of the external identity provider). Using groups being synced from external sources in `isMemberOf` will cause an error to avoid problems with [dynamic memberships](https://jackrabbit.apache.org/oak/docs/security/authentication/external/dynamic.html). Use `allowExternalGroupsInIsMemberOf: true` in `global_config` if you need to override this behaviour (should be used rarely). Since v1.9.3 | optional
 path | Path of the intermediate node either relative or absolute. If relative, `/home/groups` is automatically prefixed. By default some implementation specific path is choosen. Usually the full group path is the (intermediate) path concatenated with a [randomized authorizable id](https://jackrabbit.apache.org/oak/docs/apidocs/org/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.html).  | optional
 isMemberOf | List of groups this groups is a member of. May be provided as yaml list or as comma-separated yaml string (*the use of comma-separated yaml strings is deprecated*, available to remain backwards compatible). | optional
 memberOf | Same meaning as `isMemberOf`. This property is *deprecated*, please use `isMemberOf` instead. | optional