Skip to content

Commit

Permalink
Merge pull request #149 from apache/SLING-11663
Browse files Browse the repository at this point in the history 
SLING-11663 : Create all service users with forced path
  • Loading branch information
@anchela
anchela committed Nov 10, 2022
2 parents 18b394f + 6ec10c3 commit 684b4d7
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 60 deletions.
42 changes: 37 additions & 5 deletions src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public class DefaultAclManager implements AclManager, EnforceInfo {

private final RepoPath enforcePrincipalBasedSupportedPath;
private final String systemRelPath;
private final boolean alwaysForceSystemUserPath;

private final OperationProcessor processor = new OperationProcessor();

Expand All @@ -95,16 +96,39 @@ public class DefaultAclManager implements AclManager, EnforceInfo {

private RepoPath userRootPath;

/**
* Same as {@code DefaultAclManager(null, "system", false)}
* @see ConverterConstants#SYSTEM_USER_REL_PATH_DEFAULT
*/
public DefaultAclManager() {
this(null, ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT);
this(null, ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, false);
}

/**
* @param enforcePrincipalBasedSupportedPath The supported path if principal-based access control setup for service users should be enforced; {@code null} otherwise.
* @param systemRelPath The relative intermediate path used for all system users.
* @deprecated Use DefaultAclManager(String,String,boolean) instead
*/
@Deprecated
public DefaultAclManager(@Nullable String enforcePrincipalBasedSupportedPath, @NotNull String systemRelPath) {
this(enforcePrincipalBasedSupportedPath, systemRelPath, false);
log.warn("Deprecated. Please refactor to use DefaultAclManager(String,String,boolean) instead");
}

public DefaultAclManager(@Nullable String enforcePrincipalBasedSupportedPath, @NotNull String systemRelPath) {
/**
* Creates a new instance of {@code DefaultAclManager}.
*
* @param enforcePrincipalBasedSupportedPath The supported path if principal-based access control setup for service users should be enforced; {@code null} otherwise.
* @param systemRelPath The relative intermediate path used for all system users.
* @param alwaysForceSystemUserPath Option to make sure all system users are being created with the specified intermediate path (i.e. translating to 'with forced path' statements in repoinit).
*/
public DefaultAclManager(@Nullable String enforcePrincipalBasedSupportedPath, @NotNull String systemRelPath, boolean alwaysForceSystemUserPath) {
if (enforcePrincipalBasedSupportedPath != null && !enforcePrincipalBasedSupportedPath.contains(systemRelPath)) {
throw new RuntimeException("Relative path for system users "+ systemRelPath + " not included in " + enforcePrincipalBasedSupportedPath);
}
this.enforcePrincipalBasedSupportedPath = (enforcePrincipalBasedSupportedPath == null) ? null : new RepoPath(enforcePrincipalBasedSupportedPath);
this.systemRelPath = systemRelPath;
this.alwaysForceSystemUserPath = alwaysForceSystemUserPath;
}

@Override
Expand Down Expand Up @@ -195,15 +219,16 @@ public void addRepoinitExtention(@NotNull String source, @Nullable String repoIn
@Override
public void visitCreateServiceUser(CreateServiceUser createServiceUser) {
recordSystemUserIds(createServiceUser.getUsername());
}});
}
});
}
} catch (RepoInitParsingException e) {
throw new ConverterException(e.getMessage(), e);
}
return;
}
try (Formatter formatter = new Formatter()) {
if (enforcePrincipalBased()) {
if (enforcePrincipalBased() || alwaysForceSystemUserPath) {
List<Operation> ops = new RepoInitParserService().parse(new StringReader(repoInitText));
processor.apply(ops, formatter,this);
} else {
Expand All @@ -222,7 +247,8 @@ public void visitCreateServiceUser(CreateServiceUser createServiceUser) {
private void addUsersAndGroups(@NotNull Formatter formatter) throws ConverterException {
for (SystemUser systemUser : systemUsers) {
// make sure all system users are created first
CreateServiceUser operation = new CreateServiceUser(systemUser.getId(), new WithPathOptions(calculateIntermediatePath(systemUser), enforcePrincipalBased(systemUser)));
boolean withForcedPath = (alwaysForceSystemUserPath || enforcePrincipalBased(systemUser));
CreateServiceUser operation = new CreateServiceUser(systemUser.getId(), new WithPathOptions(calculateIntermediatePath(systemUser), withForcedPath));
formatter.format("%s", operation.asRepoInitString());
if (systemUser.getDisabledReason() != null) {
DisableServiceUser disable = new DisableServiceUser(systemUser.getId(), systemUser.getDisabledReason());
Expand Down Expand Up @@ -376,6 +402,12 @@ public boolean enforcePrincipalBased(@NotNull String systemUserId) {
return false;
}

@Override
public boolean enforcePath(@NotNull String systemUserId) {
// NOTE: defined by a global configuration option and thus the 'systemUserId' is ignored
return alwaysForceSystemUserPath;
}

@Override
@NotNull
public String calculateEnforcedIntermediatePath(@Nullable String intermediatePath) throws ConverterException {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforceInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ public interface EnforceInfo {
void recordSystemUserIds(@NotNull String... systemUserIds);

boolean enforcePrincipalBased(@NotNull String systemUserId);

boolean enforcePath(@NotNull String systemUserId);

@NotNull
String calculateEnforcedIntermediatePath(@Nullable String intermediatePath) throws ConverterException;

}
5 changes: 4 additions & 1 deletion ...rg/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ public final class ContentPackage2FeatureModelConverterLauncher implements Runna
@Option(names= {"--system-user-rel-path"}, description = "Relative path for system user as configured with Apache Jackrabbit Oak", required = false)
private String systemUserRelPath = ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT;

@Option(names = { "--enforce-system-user-path" }, description = "If this option is enabled all generated 'create service user' repoinit statements with come with 'forced path' set to make sure the intermediate path matches.", required = false)
private boolean alwaysForceSystemUserPath = false;

@Option(names = { "--enforce-servicemapping-by-principal" }, description = "Converts service user mappings with the form 'service:sub=userID' to 'service:sub=[principalname]'. Note, this may result in group membership no longer being resolved upon service login.", required = false)
private boolean enforceServiceMappingByPrincipal = false;

Expand Down Expand Up @@ -175,7 +178,7 @@ public void run() {

try {
try {
AclManager aclManager = new DefaultAclManager(enforcePrincipalBasedSupportedPath, systemUserRelPath);
AclManager aclManager = new DefaultAclManager(enforcePrincipalBasedSupportedPath, systemUserRelPath, alwaysForceSystemUserPath);

DefaultFeaturesManager featuresManager = new DefaultFeaturesManager(mergeConfigurations,
bundlesStartOrder,
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/apache/sling/feature/cpconverter/repoinit/SystemUserVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.apache.sling.feature.cpconverter.ConverterException;
import org.apache.sling.feature.cpconverter.accesscontrol.EnforceInfo;
import org.apache.sling.feature.cpconverter.shared.ConverterConstants;
import org.apache.sling.repoinit.parser.operations.CreateServiceUser;
import org.apache.sling.repoinit.parser.operations.WithPathOptions;
import org.jetbrains.annotations.NotNull;
Expand All @@ -43,6 +44,9 @@ public void visitCreateServiceUser(CreateServiceUser createServiceUser) {
if (enforceInfo.enforcePrincipalBased(id)) {
CreateServiceUser operation = new CreateServiceUser(id, new WithPathOptions(enforceInfo.calculateEnforcedIntermediatePath(path), true));
formatter.format("%s", operation.asRepoInitString());
} else if (enforceInfo.enforcePath(id)) {
CreateServiceUser operation = new CreateServiceUser(id, new WithPathOptions(ConverterConstants.getValidSystemUserPath(path), true));
formatter.format("%s", operation.asRepoInitString());
} else {
formatter.format("%s", createServiceUser.asRepoInitString());
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/apache/sling/feature/cpconverter/shared/ConverterConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@
*/
package org.apache.sling.feature.cpconverter.shared;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public final class ConverterConstants {

private ConverterConstants() {}

public static final String SYSTEM_USER_REL_PATH_DEFAULT = "system";

public static final String SLASH = "/";

public static @NotNull String getValidSystemUserPath(@Nullable String intermediatePath) {
return (intermediatePath == null || intermediatePath.isEmpty()) ? ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT : intermediatePath;
}
}
53 changes: 32 additions & 21 deletions src/test/java/org/apache/sling/feature/cpconverter/ConverterUserAndPermissionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

import static org.apache.sling.feature.cpconverter.Util.createServiceUserStatement;
import static org.apache.sling.feature.cpconverter.Util.normalize;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -115,30 +116,37 @@ public class ConverterUserAndPermissionTest extends AbstractConverterTest {
COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/.content.xml");
COMMON_NOT_EXPECTED_PATHS.add("jcr_root/home/users/system/_cq_services/demo-cp/qStDu7IQBLa95gURmer1/_rep_principalPolicy.xml");
}

private static final String SUPPORTED_PATH = "/home/users/system/cq:services";

private ContentPackage2FeatureModelConverter converter;

private final EntryHandlersManager handlersManager;
private final AclManager aclManager;
private final boolean enforcePrincipalBased;
private final String withRelPath;
private final boolean alwaysEnforcePath;

private File outputDirectory;
private FeaturesManager featuresManager;

@Parameterized.Parameters(name = "name={2}")
@Parameterized.Parameters(name = "name={3}")
public static Collection<Object[]> parameters() {
return Arrays.asList(
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, null, "Default system user rel-path"},
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, "/home/users/system/cq:services", "Default system user rel-path with enforce-principal-based ac-setup"});
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, null, false, "Default system user rel-path, enforcePath=false"},
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, null, true, "Default system user rel-path, enforcePath=true"},
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, SUPPORTED_PATH, false, "Default system user rel-path with enforce-principal-based ac-setup, enforcePath=false"},
new Object[] {ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, SUPPORTED_PATH, true, "Default system user rel-path with enforce-principal-based ac-setup, enforcePath=true"});
}

public ConverterUserAndPermissionTest(@NotNull String systemUserRelPath, @Nullable String enforcePrincipalBasedSupportedPath, @NotNull String name) throws Exception {
this.aclManager = new DefaultAclManager(enforcePrincipalBasedSupportedPath, systemUserRelPath);
public ConverterUserAndPermissionTest(@NotNull String systemUserRelPath, @Nullable String enforcePrincipalBasedSupportedPath,
boolean alwaysEnforcePath, @NotNull String name) {
this.aclManager = new DefaultAclManager(enforcePrincipalBasedSupportedPath, systemUserRelPath, alwaysEnforcePath);
this.handlersManager = new DefaultEntryHandlersManager(Collections.emptyMap(), false,
ContentPackage2FeatureModelConverter.SlingInitialContentPolicy.KEEP, new BundleSlingInitialContentExtractor(), systemUserRelPath);
this.enforcePrincipalBased = (enforcePrincipalBasedSupportedPath != null);
this.withRelPath = (enforcePrincipalBased) ? enforcePrincipalBasedSupportedPath.substring(enforcePrincipalBasedSupportedPath.indexOf(systemUserRelPath)) : systemUserRelPath;
this.alwaysEnforcePath = alwaysEnforcePath;
}

@Before
Expand Down Expand Up @@ -213,19 +221,19 @@ public void testMutlipePackagesWithServiceUsersAndAcls() throws Exception {

Set<String> notExpected2 = new HashSet<>();
notExpected1.add("jcr_root/content/_rep_policy.xml");
notExpected1.add("jcr_root/var/eventproxy/_rep_policy.xml");
notExpected1.add("jcr_root/content/");
notExpected1.add("jcr_root/var/");
notExpected1.add("jcr_root/var/eventproxy/");
notExpected1.add("jcr_root/var/eventproxy/.content.xml");
notExpected1.add("jcr_root/home/");
notExpected1.add("jcr_root/home/users/");
notExpected1.add("jcr_root/home/users/system/");
notExpected1.add("jcr_root/home/users/system/eventproxy/");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/.content.xml");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/_rep_policy.xml");
notExpected1.add("jcr_root/home/users/system/eventproxy/.content.xml");
notExpected1.add("jcr_root/var/eventproxy/_rep_policy.xml");
notExpected1.add("jcr_root/content/");
notExpected1.add("jcr_root/var/");
notExpected1.add("jcr_root/var/eventproxy/");
notExpected1.add("jcr_root/var/eventproxy/.content.xml");
notExpected1.add("jcr_root/home/");
notExpected1.add("jcr_root/home/users/");
notExpected1.add("jcr_root/home/users/system/");
notExpected1.add("jcr_root/home/users/system/eventproxy/");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/.content.xml");
notExpected1.add("jcr_root/home/users/system/eventproxy/eventproxy-service/_rep_policy.xml");
notExpected1.add("jcr_root/home/users/system/eventproxy/.content.xml");

Set<String> expected2 = new HashSet<>();
expected1.add("META-INF/MANIFEST.MF");
Expand All @@ -249,7 +257,7 @@ public void testMutlipePackagesWithServiceUsersAndAcls() throws Exception {
" end\n"), normalize(repoinit.getText()));
} else {
assertEquals(normalize(
"create service user eventproxy-service with path system/eventproxy\n" +
createServiceUserStatement(alwaysEnforcePath, "eventproxy-service", "system/eventproxy") +
" create path /content\n" +
" create path /var/eventproxy(nt:unstructured mixin rep:AccessControllable)\n" +
" set ACL for eventproxy-service\n" +
Expand All @@ -271,7 +279,7 @@ public void testMutlipePackagesWithServiceUsersAndAcls() throws Exception {
" end\n"), normalize(repoinit.getText()));
} else {
assertEquals(normalize(
" create service user eventproxy-service2 with path system/eventproxy\n" +
createServiceUserStatement(alwaysEnforcePath, "eventproxy-service2", "system/eventproxy") +
" set ACL for eventproxy-service2\n" +
" allow jcr:read,rep:write on home(eventproxy-service2)\n" +
" end\n"), normalize(repoinit.getText()));
Expand Down Expand Up @@ -483,7 +491,10 @@ private void verifyRepoInit() throws RepoInitParsingException {
return csu.isForcedPath() && csu.getPath().startsWith(withRelPath);
}));
} else {
assertFalse(createServiceUsers.stream().anyMatch(operation -> ((CreateServiceUser) operation).isForcedPath()));
boolean matchingForcedPath = (alwaysEnforcePath) ?
createServiceUsers.stream().allMatch(operation -> ((CreateServiceUser) operation).isForcedPath()) :
createServiceUsers.stream().noneMatch(operation -> ((CreateServiceUser) operation).isForcedPath());
assertTrue(matchingForcedPath);
assertEquals(2, createServiceUsers.stream().filter(operation -> ((CreateServiceUser) operation).getPath().startsWith(withRelPath+"/demo-cp")).count());
}

Expand Down
9 changes: 9 additions & 0 deletions src/test/java/org/apache/sling/feature/cpconverter/Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.sling.repoinit.parser.RepoInitParsingException;
import org.apache.sling.repoinit.parser.impl.RepoInitParserService;
import org.apache.sling.repoinit.parser.operations.Operation;
import org.jetbrains.annotations.NotNull;

import java.io.StringReader;
import java.util.Formatter;
Expand All @@ -44,4 +45,12 @@ public static String normalizeUnchecked(String repoinit) {
throw new RuntimeException(e);
}
}

public static @NotNull String createServiceUserStatement(boolean enforcePath, @NotNull String id, @NotNull String intermediatePath) {
if (enforcePath) {
return "create service user "+id+" with forced path "+intermediatePath+"\n";
} else {
return "create service user "+id+" with path "+intermediatePath+"\n";
}
}
}
Loading

0 comments on commit 684b4d7

Please sign in to comment.