This is an automated email from the ASF dual-hosted git repository.
simonetripodi pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
The following commit(s) were added to refs/heads/master by this push:
new cb03166 SLING-8573 - ACLs are missing from the converted content
packages for regular users and groups
cb03166 is described below
commit cb03166805d884549b27a02ab71e1b51eeaa4aef
Author: Simo Tripodi <[email protected]>
AuthorDate: Thu Jul 11 16:44:43 2019 +0200
SLING-8573 - ACLs are missing from the converted content packages for
regular users and groups
only detected system users will be filtered in the repoinit, all other
udetected users will be stored in the related _rep_policy.xml file
---
.../handlers/RepPolicyEntryHandler.java | 88 +++++++++++++++++-----
.../cpconverter/vltpkg/VaultPackageAssembler.java | 12 +--
.../handlers/RepPolicyEntryHandlerTest.java | 75 +++++++++++++++---
3 files changed, 140 insertions(+), 35 deletions(-)
diff --git
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
index 3015622..79995f4 100644
---
a/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
+++
b/src/main/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandler.java
@@ -19,13 +19,25 @@ package org.apache.sling.feature.cpconverter.handlers;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
+import java.util.Properties;
import java.util.Stack;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
+import javax.xml.transform.stream.StreamResult;
+
+import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.vault.fs.io.Archive;
import org.apache.jackrabbit.vault.fs.io.Archive.Entry;
import
org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter;
@@ -37,6 +49,8 @@ import org.xml.sax.SAXException;
public final class RepPolicyEntryHandler extends AbstractRegexEntryHandler {
+ private final SAXTransformerFactory saxTransformerFactory =
(SAXTransformerFactory) TransformerFactory.newInstance();
+
public RepPolicyEntryHandler() {
super("/jcr_root(.*/)_rep_policy.xml");
}
@@ -57,15 +71,27 @@ public final class RepPolicyEntryHandler extends
AbstractRegexEntryHandler {
+ "' but it does not, currently");
}
- RepPolicyParser systemUserParser = new RepPolicyParser(resourcePath,
converter.getAclManager());
- boolean accepted = false;
+ Properties format = new Properties();
+ format.put(OutputKeys.INDENT, "yes");
+ format.put(OutputKeys.ENCODING, "utf-8");
+
+ TransformerHandler handler =
saxTransformerFactory.newTransformerHandler();
+ handler.getTransformer().setOutputProperties(format);
+ StringWriter stringWriter = new StringWriter();
+ handler.setResult(new StreamResult(stringWriter));
+
+ RepPolicyParser systemUserParser = new RepPolicyParser(resourcePath,
converter.getAclManager(), handler);
+ boolean hasRejectedAcls;
try (InputStream input = archive.openInputStream(entry)) {
- accepted = systemUserParser.parse(input);
+ hasRejectedAcls = systemUserParser.parse(input);
}
- if (!accepted) {
- converter.getMainPackageAssembler().addEntry(path, archive, entry);
+ if (hasRejectedAcls) {
+ try (Reader reader = new StringReader(stringWriter.toString());
+ OutputStreamWriter writer = new
OutputStreamWriter(converter.getMainPackageAssembler().createEntry(path))) {
+ IOUtils.copy(reader, writer);
+ }
}
}
@@ -83,8 +109,6 @@ public final class RepPolicyEntryHandler extends
AbstractRegexEntryHandler {
private static final String REP_PRIVILEGES = "rep:privileges";
- private static final String[] RESTRICTIONS = new String[] {
"rep:glob", "rep:ntNames", "rep:prefixes", "rep:itemNames" };
-
private static final Map<String, String> operations = new HashMap<>();
static {
@@ -92,6 +116,8 @@ public final class RepPolicyEntryHandler extends
AbstractRegexEntryHandler {
operations.put(REP_DENY_ACE, "deny");
}
+ private static final String[] RESTRICTIONS = new String[] {
"rep:glob", "rep:ntNames", "rep:prefixes", "rep:itemNames" };
+
private static final Pattern typeIndicatorPattern =
Pattern.compile("\\{[^\\}]+\\}\\[(.+)\\]");
private final Stack<Acl> acls = new Stack<>();
@@ -100,14 +126,26 @@ public final class RepPolicyEntryHandler extends
AbstractRegexEntryHandler {
private final AclManager aclManager;
+ private final TransformerHandler handler;
+
private boolean onRepAclNode = false;
- private boolean accepted = true;
+ // ACL processing result
+ private boolean hasRejectedNodes = false;
- public RepPolicyParser(String path, AclManager aclManager) {
+ // just internal pointer for every iteration
+ private boolean processCurrentAcl = false;
+
+ public RepPolicyParser(String path, AclManager aclManager,
TransformerHandler handler) {
super(REP_ACL);
this.path = path;
this.aclManager = aclManager;
+ this.handler = handler;
+ }
+
+ @Override
+ public void startDocument() throws SAXException {
+ handler.startDocument();
}
@Override
@@ -124,41 +162,55 @@ public final class RepPolicyEntryHandler extends
AbstractRegexEntryHandler {
Acl acl = new Acl(operation, privileges, Paths.get(path));
- if (aclManager.addAcl(principalName, acl)) {
+ processCurrentAcl = aclManager.addAcl(principalName, acl);
+ if (processCurrentAcl) {
acls.add(acl);
} else {
- accepted = false;
- onRepAclNode = false;
+ hasRejectedNodes = true;
}
} else if (REP_RESTRICTIONS.equals(primaryType) &&
!acls.isEmpty()) {
- for (String restriction : RESTRICTIONS) {
- String path =
extractValue(attributes.getValue(restriction));
+ if (processCurrentAcl) {
+ for (String restriction : RESTRICTIONS) {
+ String path =
extractValue(attributes.getValue(restriction));
- if (path != null && !path.isEmpty()) {
- acls.peek().addRestriction(restriction + ',' +
path);
+ if (path != null && !path.isEmpty()) {
+ acls.peek().addRestriction(restriction + ',' +
path);
+ }
}
}
}
} else {
super.startElement(uri, localName, qName, attributes);
}
+
+ if (!onRepAclNode || !processCurrentAcl) {
+ handler.startElement(uri, localName, qName, attributes);
+ }
}
@Override
public void endElement(String uri, String localName, String qName)
throws SAXException {
- if (onRepAclNode && !acls.isEmpty()) {
+ if (onRepAclNode && processCurrentAcl && !acls.isEmpty()) {
acls.pop();
+ } else {
+ processCurrentAcl = false;
+ handler.endElement(uri, localName, qName);
}
}
@Override
+ public void endDocument() throws SAXException {
+ handler.endDocument();
+ }
+
+ @Override
protected void onJcrRootElement(String uri, String localName, String
qName, Attributes attributes) {
onRepAclNode = true;
}
@Override
protected Boolean getParsingResult() {
- return accepted;
+ return hasRejectedNodes;
}
private static String extractValue(String expression) {
diff --git
a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
index 51d2107..7ec08de 100644
---
a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
+++
b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/VaultPackageAssembler.java
@@ -155,15 +155,17 @@ public class VaultPackageAssembler implements
EntryHandler {
}
public void addEntry(String path, InputStream input) throws IOException {
- File target = new File(storingDirectory, path);
-
- target.getParentFile().mkdirs();
-
- try (OutputStream output = new FileOutputStream(target)) {
+ try (OutputStream output = createEntry(path)) {
IOUtils.copy(input, output);
}
}
+ public OutputStream createEntry(String path) throws IOException {
+ File target = new File(storingDirectory, path);
+ target.getParentFile().mkdirs();
+ return new FileOutputStream(target);
+ }
+
public File getEntry(String path) {
if (!path.startsWith(ROOT_DIR)) {
path = ROOT_DIR + path;
diff --git
a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
index 7bd8fc6..5545bb4 100644
---
a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
+++
b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java
@@ -26,6 +26,7 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.StringReader;
import java.nio.file.Path;
@@ -89,7 +90,7 @@ public final class RepPolicyEntryHandlerTest {
"acs-commons-package-replication-status-event-service",
"acs-commons-ensure-service-user-service",
"acs-commons-automatic-package-replicator-service",
-
"acs-commons-on-deploy-scripts-service");
+
"acs-commons-on-deploy-scripts-service").getRepoinitExtension();
assertNotNull(repoinitExtension);
assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
@@ -130,10 +131,12 @@ public final class RepPolicyEntryHandlerTest {
@Test
public void notDeclaredSystemUsersWillNotHaveAclSettings() throws
Exception {
- Extension repoinitExtension =
parseAndSetRepoinit("acs-commons-package-replication-status-event-service",
-
"acs-commons-ensure-service-user-service",
-
"acs-commons-automatic-package-replicator-service",
-
"acs-commons-on-deploy-scripts-service");
+ ParseResult result =
parseAndSetRepoinit("acs-commons-package-replication-status-event-service",
+
"acs-commons-ensure-service-user-service",
+
"acs-commons-automatic-package-replicator-service",
+
"acs-commons-on-deploy-scripts-service");
+ Extension repoinitExtension = result.getRepoinitExtension();
+
assertNotNull(repoinitExtension);
assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
@@ -162,11 +165,22 @@ public final class RepPolicyEntryHandlerTest {
RepoInitParser repoInitParser = new RepoInitParserService();
List<Operation> operations = repoInitParser.parse(new
StringReader(actual));
assertFalse(operations.isEmpty());
+
+ // acs-commons-ensure-oak-index-service and
acs-commons-dispatcher-flush-service not recognized as system users
+ expected = "<?xml version=\"1.0\" encoding=\"utf-8\"?><jcr:root
xmlns:jcr=\"http://www.jcp.org/jcr/1.0\"; xmlns:rep=\"internal\"
jcr:primaryType=\"rep:ACL\">\n" +
+ "<allow0 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-ensure-oak-index-service\"
rep:privileges=\"{Name}[jcr:read,rep:write,rep:indexDefinitionManagement]\">\n"
+
+ "<rep:restrictions jcr:primaryType=\"rep:Restrictions\"
rep:glob=\"{Name}[*/oak:index/*]\"/>\n" +
+ "</allow0>\n" +
+ "<allow1 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-dispatcher-flush-service\"
rep:privileges=\"{Name}[jcr:read,crx:replicate,jcr:removeNode]\"/>\n" +
+ "</jcr:root>\n";
+ actual = result.getExcludedAcls();
+ assertEquals(expected, actual);
}
@Test
public void systemUserAclSetNotForUserPath() throws Exception {
- Extension repoinitExtension = parseAndSetRepoinit(new
SystemUser("acs-commons-package-replication-status-event-service",
Paths.get("/this/is/a/completely/different/path")));
+ ParseResult result = parseAndSetRepoinit(new
SystemUser("acs-commons-package-replication-status-event-service",
Paths.get("/this/is/a/completely/different/path")));
+ Extension repoinitExtension = result.getRepoinitExtension();
assertNotNull(repoinitExtension);
assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
@@ -182,15 +196,28 @@ public final class RepPolicyEntryHandlerTest {
RepoInitParser repoInitParser = new RepoInitParserService();
List<Operation> operations = repoInitParser.parse(new
StringReader(actual));
assertFalse(operations.isEmpty());
+
+ // acs-commons-package-replication-status-event-service only
recognised as system user - ACLs in allow2
+ expected = "<?xml version=\"1.0\" encoding=\"utf-8\"?><jcr:root
xmlns:jcr=\"http://www.jcp.org/jcr/1.0\"; xmlns:rep=\"internal\"
jcr:primaryType=\"rep:ACL\">\n" +
+ "<allow0 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-ensure-oak-index-service\"
rep:privileges=\"{Name}[jcr:read,rep:write,rep:indexDefinitionManagement]\">\n"
+
+ "<rep:restrictions jcr:primaryType=\"rep:Restrictions\"
rep:glob=\"{Name}[*/oak:index/*]\"/>\n" +
+ "</allow0>\n" +
+ "<allow1 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-dispatcher-flush-service\"
rep:privileges=\"{Name}[jcr:read,crx:replicate,jcr:removeNode]\"/>\n" +
+ "<allow3 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-ensure-service-user-service\"
rep:privileges=\"{Name}[jcr:read,rep:write,jcr:readAccessControl,jcr:modifyAccessControl]\"/>\n"
+
+ "<allow4 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-automatic-package-replicator-service\"
rep:privileges=\"{Name}[jcr:read]\"/>\n" +
+ "<allow5 jcr:primaryType=\"rep:GrantACE\"
rep:principalName=\"acs-commons-on-deploy-scripts-service\"
rep:privileges=\"{Name}[jcr:read]\"/>\n" +
+ "</jcr:root>\n";
+ actual = result.getExcludedAcls();
+ assertEquals(expected, actual);
}
@Test
public void parseEmptyAcl() throws Exception {
- Extension repoinitExtension = parseAndSetRepoinit(new String[] {});
+ Extension repoinitExtension = parseAndSetRepoinit(new String[]
{}).getRepoinitExtension();
assertNull(repoinitExtension);
}
- private Extension parseAndSetRepoinit(String...systemUsersNames) throws
Exception {
+ private ParseResult parseAndSetRepoinit(String...systemUsersNames) throws
Exception {
Path alwaysTheSamePath = Paths.get("/asd/public");
SystemUser[] systemUsers = new SystemUser[systemUsersNames.length];
@@ -201,11 +228,13 @@ public final class RepPolicyEntryHandlerTest {
return parseAndSetRepoinit(systemUsers);
}
- private Extension parseAndSetRepoinit(SystemUser...systemUsers) throws
Exception {
+ private ParseResult parseAndSetRepoinit(SystemUser...systemUsers) throws
Exception {
String path = "/jcr_root/asd/public/_rep_policy.xml";
Archive archive = mock(Archive.class);
Entry entry = mock(Entry.class);
VaultPackageAssembler packageAssembler =
mock(VaultPackageAssembler.class);
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ when(packageAssembler.createEntry(anyString())).thenReturn(baos);
when(archive.openInputStream(entry)).thenReturn(getClass().getResourceAsStream(path.substring(1)));
@@ -215,8 +244,7 @@ public final class RepPolicyEntryHandlerTest {
ContentPackage2FeatureModelConverter converter =
spy(ContentPackage2FeatureModelConverter.class);
when(converter.getFeaturesManager()).thenReturn(featuresManager);
when(converter.getAclManager()).thenReturn(new DefaultAclManager());
-
- handler.handle(path, archive, entry, converter);
+ when(converter.getMainPackageAssembler()).thenReturn(packageAssembler);
if (systemUsers != null) {
for (SystemUser systemUser : systemUsers) {
@@ -224,10 +252,33 @@ public final class RepPolicyEntryHandlerTest {
}
}
+ handler.handle(path, archive, entry, converter);
+
when(packageAssembler.getEntry(anyString())).thenReturn(new
File("itdoesnotexist"));
converter.getAclManager().addRepoinitExtension(Arrays.asList(packageAssembler),
feature);
- return
feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT);
+ return new
ParseResult(feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT),
new String(baos.toByteArray()));
+ }
+
+ private static final class ParseResult {
+
+ private final Extension repoinitExtension;
+
+ private final String excludedAcls;
+
+ public ParseResult(Extension repoinitExtension, String excludedAcls) {
+ this.repoinitExtension = repoinitExtension;
+ this.excludedAcls = excludedAcls;
+ }
+
+ public Extension getRepoinitExtension() {
+ return repoinitExtension;
+ }
+
+ public String getExcludedAcls() {
+ return excludedAcls;
+ }
+
}
}
