Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SLING-11663 : Create all service users with forced path #149

Merged
merged 2 commits into from
Nov 10, 2022
Merged

SLING-11663 : Create all service users with forced path #149

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1788af5
SLING-11663 : Create all service users with forced path
anchela Nov 4, 2022
6ec10c3
SLING-11663 : avoid removing existing constructor but instead depreca…
anchela Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
View file
Open in desktop
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 @@ -96,15 +97,16 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
private RepoPath userRootPath;

public DefaultAclManager() {
this(null, ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT);
this(null, ConverterConstants.SYSTEM_USER_REL_PATH_DEFAULT, false);
}

public DefaultAclManager(@Nullable String enforcePrincipalBasedSupportedPath, @NotNull String systemRelPath) {
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 +197,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 +225,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 +380,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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";
}
}
}