SLING-10070 : Option to enforce principal-based authorization (drop redundant boolean config option)
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
index 697b0ff..e898d13 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java
@@ -56,8 +56,7 @@
private static final String CONTENT_XML_FILE_NAME = ".content.xml";
- private final boolean enforcePrincipalBased;
- private final RepoPath supportedPrincipalBasedPath;
+ private final RepoPath enforcePrincipalBasedSupportedPath;
private final Set<SystemUser> systemUsers = new LinkedHashSet<>();
private final Set<Group> groups = new LinkedHashSet<>();
@@ -72,11 +71,10 @@
private volatile PrivilegeDefinitions privilegeDefinitions;
public DefaultAclManager() {
- this(false, null);
+ this(null);
}
- public DefaultAclManager(boolean enforcePrincipalBased, @Nullable String supportedPrincipalBasedPath) {
- this.enforcePrincipalBased = enforcePrincipalBased;
- this.supportedPrincipalBasedPath = (supportedPrincipalBasedPath == null) ? null : new RepoPath(supportedPrincipalBasedPath);
+ public DefaultAclManager(@Nullable String enforcePrincipalBasedSupportedPath) {
+ this.enforcePrincipalBasedSupportedPath = (enforcePrincipalBasedSupportedPath == null) ? null : new RepoPath(enforcePrincipalBasedSupportedPath);
}
@Override
@@ -172,16 +170,16 @@
@NotNull
private String calculateIntermediatePath(@NotNull SystemUser systemUser) {
RepoPath intermediatePath = systemUser.getIntermediatePath();
- if (enforcePrincipalBased(systemUser) && supportedPrincipalBasedPath != null && !intermediatePath.startsWith(supportedPrincipalBasedPath)) {
+ if (enforcePrincipalBased(systemUser) && !intermediatePath.startsWith(enforcePrincipalBasedSupportedPath)) {
RepoPath parent = intermediatePath.getParent();
while (parent != null) {
- if (supportedPrincipalBasedPath.startsWith(parent)) {
+ if (enforcePrincipalBasedSupportedPath.startsWith(parent)) {
String relpath = intermediatePath.toString().substring(parent.toString().length());
- return supportedPrincipalBasedPath.toString() + relpath;
+ return enforcePrincipalBasedSupportedPath.toString() + relpath;
}
parent = parent.getParent();
}
- throw new IllegalStateException("Cannot calculate intermediate path for service user. Configured Supported path " +supportedPrincipalBasedPath+" has no common ancestor with "+intermediatePath);
+ throw new IllegalStateException("Cannot calculate intermediate path for service user. Configured Supported path " +enforcePrincipalBasedSupportedPath+" has no common ancestor with "+intermediatePath);
} else {
return intermediatePath.toString();
}
@@ -249,7 +247,7 @@
}
private boolean enforcePrincipalBased(@NotNull SystemUser systemUser) {
- if (!enforcePrincipalBased) {
+ if (enforcePrincipalBasedSupportedPath == null) {
return false;
} else {
if (mappedById.contains(systemUser.getId())) {
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 73dd0c2..94332af 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
@@ -96,11 +96,8 @@
@Option(names = { "-Z", "--fail-on-mixed-packages" }, description = "Fail the conversion if the resulting attached content-package is MIXED type", required = false)
private boolean failOnMixedPackages = false;
- @Option(names = { "--enforce-principal-based" }, description = "Converts all service user access control entries to principal-based setup", required = false)
- private boolean enforcePrincipalBased = false;
-
- @Option(names = { "--supported-principal-based-path" }, description = "Path supported for principal-based access control setup", required = false)
- private String supportedPrincipalBasedPath = null;
+ @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 = { "--entry-handler-config" }, description = "Config for entry handlers that support it (classname:<config-string>", required = false)
private List<String> entryHandlerConfigs = null;
@@ -161,7 +158,7 @@
.setFeaturesManager(featuresManager)
.setBundlesDeployer(new DefaultArtifactsDeployer(artifactsOutputDirectory))
.setEntryHandlersManager(new DefaultEntryHandlersManager(entryHandlerConfigsMap))
- .setAclManager(new DefaultAclManager(enforcePrincipalBased, supportedPrincipalBasedPath))
+ .setAclManager(new DefaultAclManager(enforcePrincipalBasedSupportedPath))
.setEmitter(DefaultPackagesEventsEmitter.open(featureModelsOutputDirectory))
.setFailOnMixedPackages(failOnMixedPackages)
.setDropContent(true);
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
index dcc438d..13955dc 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java
@@ -64,7 +64,7 @@
@Before
public void setUp() throws Exception {
- aclManager = new DefaultAclManager(true, "/home/users/system/some/subtree");
+ aclManager = new DefaultAclManager("/home/users/system/some/subtree");
tempDir = Files.createTempDirectory(getClass().getSimpleName());
assembler = mock(VaultPackageAssembler.class);
@@ -88,24 +88,14 @@
@Test(expected = IllegalStateException.class)
public void testInvalidSupportedPath() {
- AclManager acMgr = new DefaultAclManager(true, "/an/invalid/supported/path");
+ AclManager acMgr = new DefaultAclManager("/an/invalid/supported/path");
RepoPath accessControlledPath = new RepoPath("/content/feature");
getRepoInitExtension(acMgr, accessControlledPath, systemUser, false);
}
@Test
- public void testMissingSupportedPath() {
- AclManager acMgr = new DefaultAclManager(true, null);
- RepoPath accessControlledPath = new RepoPath("/content/feature");
- String txt = getRepoInitExtension(acMgr, accessControlledPath, systemUser, false).getText();
-
- assertFalse(txt.contains("create service user user1 with path "+remappedIntermediatePath));
- assertTrue(txt.contains("create service user user1 with path " + systemUser.getIntermediatePath()));
- }
-
- @Test
public void testResourceBasedConversionWithoutForce() throws RepoInitParsingException {
- AclManager acMgr = new DefaultAclManager(false, "/home/users/system/some/subtree"){
+ AclManager acMgr = new DefaultAclManager(null) {
@Override
protected @Nullable String computePathWithTypes(@NotNull RepoPath path, @NotNull List<VaultPackageAssembler> packageAssemblers) {
return "/content/feature(sling:Folder)";
@@ -117,10 +107,10 @@
String expected =
"create service user user1 with path " + systemUser.getIntermediatePath() + System.lineSeparator() +
- "create path /content/feature(sling:Folder)" + System.lineSeparator() +
- "set ACL for user1" + System.lineSeparator() +
- "allow jcr:read on /content/feature" + System.lineSeparator() +
- "end" + System.lineSeparator();
+ "create path /content/feature(sling:Folder)" + System.lineSeparator() +
+ "set ACL for user1" + System.lineSeparator() +
+ "allow jcr:read on /content/feature" + System.lineSeparator() +
+ "end" + System.lineSeparator();
String actual = repoinitExtension.getText();
assertEquals(expected, actual);
@@ -128,25 +118,6 @@
RepoInitParser repoInitParser = new RepoInitParserService();
List<Operation> operations = repoInitParser.parse(new StringReader(actual));
assertFalse(operations.isEmpty());
-
- aclManager.addSystemUser(systemUser);
- feature.getExtensions().clear();
-
- accessControlledPath = new RepoPath("/content/feature");
- repoinitExtension = getRepoInitExtension(aclManager, accessControlledPath, systemUser, false);
-
- expected =
- "create service user user1 with path " + remappedIntermediatePath + System.lineSeparator() +
- "set principal ACL for user1" + System.lineSeparator() +
- "allow jcr:read on /content/feature" + System.lineSeparator() +
- "end" + System.lineSeparator();
-
- actual = repoinitExtension.getText();
- assertEquals(expected, actual);
-
- repoInitParser = new RepoInitParserService();
- operations = repoInitParser.parse(new StringReader(actual));
- assertFalse(operations.isEmpty());
}
@Test
