This is an automated email from the ASF dual-hosted git repository.

angela pushed a commit to branch SLING-10210
in repository 
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git

commit 110898c275b6bddd9ffdc5df966f662cac91aa67
Author: angela <[email protected]>
AuthorDate: Fri Mar 12 11:54:17 2021 +0100

    SLING-10210 : Option to enforce service-user-mapping with principal names
---
 .../feature/cpconverter/accesscontrol/Mapping.java | 39 ++++++++++++++++++--
 ...ntentPackage2FeatureModelConverterLauncher.java |  5 ++-
 .../AbstractConfigurationEntryHandler.java         | 17 ++++++++-
 .../handlers/DefaultEntryHandlersManager.java      |  7 ++--
 .../cpconverter/accesscontrol/MappingTest.java     | 42 +++++++++++++++++-----
 5 files changed, 95 insertions(+), 15 deletions(-)

diff --git 
a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/Mapping.java 
b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/Mapping.java
index 9b91372..fb0d659 100644
--- 
a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/Mapping.java
+++ 
b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/Mapping.java
@@ -18,6 +18,8 @@ package org.apache.sling.feature.cpconverter.accesscontrol;
 
 import org.jetbrains.annotations.NotNull;
 
+import java.util.Collections;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.Objects;
 import java.util.Set;
@@ -32,10 +34,14 @@ public class Mapping {
 
     private final Set<String> principalNames;
 
-    /**
-     * Copied from 
https://github.com/apache/sling-org-apache-sling-serviceusermapper/blob/master/src/main/java/org/apache/sling/serviceusermapping/Mapping.java
-     */
     public Mapping(@NotNull final String spec) {
+        this(spec, false);
+    }
+
+        /**
+         * Copied from 
https://github.com/apache/sling-org-apache-sling-serviceusermapper/blob/master/src/main/java/org/apache/sling/serviceusermapping/Mapping.java
+         */
+    public Mapping(@NotNull final String spec, boolean 
enforceMappingByPrincipal) {
 
         final int colon = spec.indexOf(':');
         final int equals = spec.indexOf('=');
@@ -60,6 +66,9 @@ public class Mapping {
         if (s.charAt(0) == '[' && s.charAt(s.length()-1) == ']') {
             this.userName = null;
             this.principalNames = extractPrincipalNames(s);
+        } else if (enforceMappingByPrincipal) {
+            this.userName = null;
+            this.principalNames = Collections.singleton(s);
         } else {
             this.userName = s;
             this.principalNames = null;
@@ -127,4 +136,28 @@ public class Mapping {
         return "Mapping [serviceName=" + serviceName + ", subServiceName="
                 + subServiceName + ", " + name;
     }
+
+    @NotNull
+    public String asString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append(serviceName);
+        if (subServiceName != null) {
+            sb.append(':').append(subServiceName);
+        }
+        sb.append('=');
+        if (userName != null) {
+            sb.append(userName);
+        } else {
+            sb.append('[');
+            Iterator<String> it = principalNames.iterator();
+            while (it.hasNext()) {
+                sb.append(it.next());
+                if (it.hasNext()) {
+                    sb.append(',');
+                }
+            }
+            sb.append(']');
+        }
+        return sb.toString();
+    }
 }
\ No newline at end of file
diff --git 
a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
 
b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
index 61dd3c5..ecf1fc9 100644
--- 
a/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
+++ 
b/src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
@@ -101,6 +101,9 @@ public final class 
ContentPackage2FeatureModelConverterLauncher implements Runna
     @Option(names = { "--enforce-principal-based-supported-path" }, 
description = "Converts service user access control entries to principal-based 
setup using the given supported path.", required = false)
     private String enforcePrincipalBasedSupportedPath = null;
 
+    @Option(names = { "--enforce-servicemapping-by-principal" }, description = 
"Converts service user mappings with the form 'service:sub=userID' to 
'service:sub=[principalname]'.", required = false)
+    private boolean enforceServiceMappingByPrincipal = false;
+
     @Option(names = { "--entry-handler-config" }, description = "Config for 
entry handlers that support it (classname:<config-string>", required = false)
     private List<String> entryHandlerConfigs = null;
 
@@ -161,7 +164,7 @@ public final class 
ContentPackage2FeatureModelConverterLauncher implements Runna
                 ContentPackage2FeatureModelConverter converter = new 
ContentPackage2FeatureModelConverter(strictValidation)
                                                                 
.setFeaturesManager(featuresManager)
                                                                 
.setBundlesDeployer(new DefaultArtifactsDeployer(artifactsOutputDirectory))
-                                                                
.setEntryHandlersManager(new 
DefaultEntryHandlersManager(entryHandlerConfigsMap))
+                                                                
.setEntryHandlersManager(new 
DefaultEntryHandlersManager(entryHandlerConfigsMap, 
enforceServiceMappingByPrincipal))
                                                                 
.setAclManager(new DefaultAclManager(enforcePrincipalBasedSupportedPath))
                                                                 
.setEmitter(DefaultPackagesEventsEmitter.open(featureModelsOutputDirectory))
                                                                 
.setFailOnMixedPackages(failOnMixedPackages)
diff --git 
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractConfigurationEntryHandler.java
 
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractConfigurationEntryHandler.java
index 29f7eaf..22bb540 100644
--- 
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractConfigurationEntryHandler.java
+++ 
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractConfigurationEntryHandler.java
@@ -17,7 +17,9 @@
 package org.apache.sling.feature.cpconverter.handlers;
 
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Dictionary;
+import java.util.List;
 import java.util.Objects;
 import java.util.regex.Matcher;
 
@@ -39,10 +41,16 @@ abstract class AbstractConfigurationEntryHandler extends 
AbstractRegexEntryHandl
 
     private static final String SERVICE_USER_MAPPING_PID = 
"org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl";
 
+    private boolean enforceServiceMappingByPrincipal;
+
     public AbstractConfigurationEntryHandler(@NotNull String extension) {
         
super("/jcr_root/(?:apps|libs)/.+/config(\\.(?<runmode>[^/]+))?/(?<pid>.*)\\." 
+ extension);
     }
 
+    void setEnforceServiceMappingByPrincipal(boolean 
enforceServiceMappingByPrincipal) {
+        this.enforceServiceMappingByPrincipal = 
enforceServiceMappingByPrincipal;
+    }
+
     @Override
     public final void handle(@NotNull String path, @NotNull Archive archive, 
@NotNull Entry entry, @NotNull ContentPackage2FeatureModelConverter converter) 
throws Exception {
 
@@ -103,8 +111,15 @@ abstract class AbstractConfigurationEntryHandler extends 
AbstractRegexEntryHandl
                 String[] mappings = 
Converters.standardConverter().convert(configurationProperties.get("user.mapping")).to(String[].class);
                 if (mappings != null) {
                     AclManager aclManager = 
Objects.requireNonNull(converter.getAclManager());
+                    List<String> newMappings = new ArrayList<>();
                     for (String usermapping : mappings) {
-                        aclManager.addMapping(new Mapping(usermapping));
+                        Mapping mapping = new Mapping(usermapping, 
enforceServiceMappingByPrincipal);
+                        aclManager.addMapping(mapping);
+                        newMappings.add(mapping.asString());
+                    }
+                    // replace 'user.mapping' property by the new mappings, 
which may have been refactored
+                    if (!newMappings.isEmpty()) {
+                        configurationProperties.put("user.mapping", 
newMappings.toArray(new String[0]));
                     }
                 }
                 featuresManager.addConfiguration(runMode, id, path, 
configurationProperties);
diff --git 
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java
 
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java
index af4b375..7169489 100644
--- 
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java
+++ 
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java
@@ -30,14 +30,17 @@ public class DefaultEntryHandlersManager implements 
EntryHandlersManager {
     private final List<EntryHandler> entryHandlers = new LinkedList<>();
 
     public DefaultEntryHandlersManager() {
-        this(Collections.emptyMap());
+        this(Collections.emptyMap(), false);
     }
 
-    public DefaultEntryHandlersManager(Map<String, String> configs) {
+    public DefaultEntryHandlersManager(@NotNull Map<String, String> configs, 
boolean enforceServiceMappingByPrincipal) {
         ServiceLoader<EntryHandler> entryHandlersLoader = 
ServiceLoader.load(EntryHandler.class);
         for (EntryHandler entryHandler : entryHandlersLoader) {
             if (configs.containsKey(entryHandler.getClass().getName())) {
                 entryHandler = 
entryHandler.withConfig(configs.get(entryHandler.getClass().getName()));
+                if (entryHandler instanceof AbstractConfigurationEntryHandler) 
{
+                    ((AbstractConfigurationEntryHandler) 
entryHandler).setEnforceServiceMappingByPrincipal(enforceServiceMappingByPrincipal);
+                }
             }
             addEntryHandler(entryHandler);
         }
diff --git 
a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/MappingTest.java
 
b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/MappingTest.java
index 96b5242..d139597 100644
--- 
a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/MappingTest.java
+++ 
b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/MappingTest.java
@@ -27,67 +27,93 @@ public class MappingTest {
 
     @Test
     public void testMapUserId() {
-        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service-1=service1");
+        String spec = "org.apache.sling.testbundle:sub-service-1=service1";
+        Mapping m = new Mapping(spec);
         assertTrue(m.mapsUser("service1"));
         assertFalse(m.mapsUser("another"));
         assertFalse(m.mapsPrincipal("service1"));
         assertFalse(m.mapsPrincipal("another"));
+        assertEquals(spec, m.asString());
+    }
+
+    @Test
+    public void testMapUserIdEnforcePrincipal() {
+        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service-1=service1", true);
+        assertFalse(m.mapsUser("service1"));
+        assertFalse(m.mapsUser("another"));
+        assertTrue(m.mapsPrincipal("service1"));
+        assertFalse(m.mapsPrincipal("another"));
+        assertEquals("org.apache.sling.testbundle:sub-service-1=[service1]", 
m.asString());
     }
 
     @Test
     public void testMapPrincipalNames() {
-        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service-1=[service1,service2]");
+        String spec = 
"org.apache.sling.testbundle:sub-service-1=[service1,service2]";
+        Mapping m = new Mapping(spec);
         assertFalse(m.mapsUser("service1"));
         assertFalse(m.mapsUser("another"));
         assertTrue(m.mapsPrincipal("service1"));
         assertTrue(m.mapsPrincipal("service2"));
         assertFalse(m.mapsPrincipal("another"));
+        assertEquals(spec, m.asString());
     }
 
     @Test
     public void testMapSinglePrincipalName() {
-        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service-1=[service1]");
+        String spec = "org.apache.sling.testbundle:sub-service-1=[service1]";
+        Mapping m = new Mapping(spec);
         assertFalse(m.mapsUser("service1"));
         assertFalse(m.mapsUser("another"));
         assertTrue(m.mapsPrincipal("service1"));
         assertFalse(m.mapsPrincipal("service2"));
         assertFalse(m.mapsPrincipal("another"));
+        assertEquals(spec, m.asString());
     }
 
     @Test
     public void testMapEmptyPrincipalNames() {
-        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service-1=[]");
+        String spec = "org.apache.sling.testbundle:sub-service-1=[]";
+        Mapping m = new Mapping(spec);
         assertFalse(m.mapsUser("service1"));
         assertFalse(m.mapsUser("another"));
         assertFalse(m.mapsPrincipal("service1"));
         assertFalse(m.mapsPrincipal("service2"));
         assertFalse(m.mapsPrincipal("another"));
+        assertEquals(spec, m.asString());
     }
 
     @Test
     public void testMapMissingSubservice() {
-        Mapping m = new Mapping("org.apache.sling.testbundle=[service1]");
+        String spec = "org.apache.sling.testbundle=[service1]";
+        Mapping m = new Mapping(spec);
         assertFalse(m.mapsUser("service1"));
         assertFalse(m.mapsUser("another"));
         assertTrue(m.mapsPrincipal("service1"));
         assertFalse(m.mapsPrincipal("another"));
+        assertEquals(spec, m.asString());
     }
 
     @Test
     public void testMapIncompleteArray() {
-        Mapping m = new 
Mapping("org.apache.sling.testbundle:sub-service=[service1");
+        String spec = "org.apache.sling.testbundle:sub-service=[service1";
+        Mapping m = new Mapping(spec);
         assertTrue(m.mapsUser("[service1"));
         assertFalse(m.mapsPrincipal("service1"));
+        assertEquals(spec, m.asString());
 
-        m = new Mapping("org.apache.sling.testbundle:sub-service=service1]");
+        spec = "org.apache.sling.testbundle:sub-service=service1]";
+        m = new Mapping(spec);
         assertTrue(m.mapsUser("service1]"));
         assertFalse(m.mapsPrincipal("service1"));
+        assertEquals(spec, m.asString());
     }
 
     @Test
     public void testColonInUserName() {
-        Mapping m = new Mapping("org.apache.sling.testbundle=sling:service1");
+        String spec = "org.apache.sling.testbundle=sling:service1";
+        Mapping m = new Mapping(spec);
         assertTrue(m.mapsUser("sling:service1"));
+        assertEquals(spec, m.asString());
     }
 
     @Test(expected = IllegalArgumentException.class)

Reply via email to