Merge pull request #73 from apache/SLING-10286
SLING-10286 : Converter ignores disabled status of system user
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AbstractUser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AbstractUser.java
index 93c93c1..202dc44 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AbstractUser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AbstractUser.java
@@ -18,6 +18,7 @@
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
import java.util.Objects;
@@ -27,6 +28,8 @@
private final RepoPath path;
private final RepoPath intermediatePath;
+
+ private final String disabledReason;
/**
* @param id - the authorizableId to use.
@@ -34,9 +37,14 @@
* @param intermediatePath - the intermediate path the user should have - most likely the (direct) parent of the path.
*/
protected AbstractUser(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath) {
+ this(id, path, intermediatePath, null);
+ }
+
+ protected AbstractUser(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath, @Nullable String disabledReason) {
this.id = id;
this.path = path;
this.intermediatePath = intermediatePath;
+ this.disabledReason = disabledReason;
}
public @NotNull String getId() {
@@ -50,6 +58,10 @@
public @NotNull RepoPath getIntermediatePath() {
return intermediatePath;
}
+
+ public @Nullable String getDisabledReason() {
+ return disabledReason;
+ }
@Override
public int hashCode() {
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 f681e6e..c20118e 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
@@ -31,6 +31,7 @@
import org.apache.sling.repoinit.parser.RepoInitParsingException;
import org.apache.sling.repoinit.parser.impl.RepoInitParserService;
import org.apache.sling.repoinit.parser.operations.CreateServiceUser;
+import org.apache.sling.repoinit.parser.operations.DisableServiceUser;
import org.apache.sling.repoinit.parser.operations.Operation;
import org.apache.sling.repoinit.parser.impl.WithPathOptions;
import org.apache.sling.repoinit.parser.operations.AclLine;
@@ -217,6 +218,10 @@
// make sure all system users are created first
CreateServiceUser operation = new CreateServiceUser(systemUser.getId(), new WithPathOptions(calculateIntermediatePath(systemUser), enforcePrincipalBased(systemUser)));
formatter.format("%s", operation.asRepoInitString());
+ if (systemUser.getDisabledReason() != null) {
+ DisableServiceUser disable = new DisableServiceUser(systemUser.getId(), systemUser.getDisabledReason());
+ formatter.format("%s", disable.asRepoInitString());
+ }
if (aclIsBelow(systemUser.getPath())) {
throw new IllegalStateException("Detected policy on subpath of system-user: " + systemUser);
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/SystemUser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/SystemUser.java
index e03eda1..5eec329 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/SystemUser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/SystemUser.java
@@ -18,6 +18,7 @@
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
public class SystemUser extends AbstractUser {
@@ -29,4 +30,14 @@
public SystemUser(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath) {
super(id, path, intermediatePath);
}
+
+ /**
+ * @param id - the authorizableId to use.
+ * @param path - the original repository path of the user in the content-package.
+ * @param intermediatePath - the intermediate path the user should have - most likely the (direct) parent of the path.
+ * @param disabledReason - the reason why this user has been disabled or {@code null}.
+ */
+ public SystemUser(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath, @Nullable String disabledReason) {
+ super(id, path, intermediatePath, disabledReason);
+ }
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/User.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/User.java
index cad22bb..c864d72 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/User.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/User.java
@@ -18,6 +18,7 @@
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
public class User extends AbstractUser {
@@ -29,4 +30,14 @@
public User(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath) {
super(id, path, intermediatePath);
}
+
+ /**
+ * @param id - the authorizableId to use.
+ * @param path - the original repository path of the user in the content-package.
+ * @param intermediatePath - the intermediate path the user should have - most likely the (direct) parent of the path.
+ * @param disabledReason - the reason why this user has been disabled or {@code null}.
+ */
+ public User(@NotNull String id, @NotNull RepoPath path, @NotNull RepoPath intermediatePath, @Nullable String disabledReason) {
+ super(id, path, intermediatePath, disabledReason);
+ }
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserParser.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserParser.java
index 3d360c5..354e9fa 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserParser.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractUserParser.java
@@ -49,7 +49,7 @@
protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) {
String authorizableId = attributes.getValue(REP_AUTHORIZABLE_ID);
if (authorizableId != null && !authorizableId.isEmpty()) {
- handleUser(authorizableId);
+ handleUser(authorizableId, attributes);
}
}
@@ -58,6 +58,6 @@
return null;
}
- abstract void handleUser(@NotNull String id);
+ abstract void handleUser(@NotNull String id, @NotNull Attributes attributes);
}
\ No newline at end of file
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/GroupEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/GroupEntryHandler.java
index 7d1b2b2..68a2d38 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/GroupEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/GroupEntryHandler.java
@@ -20,6 +20,7 @@
import org.apache.sling.feature.cpconverter.accesscontrol.Group;
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.xml.sax.Attributes;
public final class GroupEntryHandler extends AbstractUserEntryHandler {
@@ -55,7 +56,7 @@
}
@Override
- void handleUser(@NotNull String id) {
+ void handleUser(@NotNull String id, @NotNull Attributes attributes) {
converter.getAclManager().addGroup(new Group(id, path, intermediatePath));
}
}
diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandler.java
index 3291415..c55f9f7 100644
--- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandler.java
+++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandler.java
@@ -21,6 +21,7 @@
import org.apache.sling.feature.cpconverter.accesscontrol.User;
import org.apache.sling.feature.cpconverter.shared.RepoPath;
import org.jetbrains.annotations.NotNull;
+import org.xml.sax.Attributes;
public final class UsersEntryHandler extends AbstractUserEntryHandler {
@@ -39,10 +40,10 @@
@Override
AbstractUserParser createParser(@NotNull ContentPackage2FeatureModelConverter converter, @NotNull RepoPath originalPath, @NotNull RepoPath intermediatePath) {
- return new SystemUserParser(converter, originalPath, intermediatePath);
+ return new UserParser(converter, originalPath, intermediatePath);
}
- private static final class SystemUserParser extends AbstractUserParser {
+ private static final class UserParser extends AbstractUserParser {
private static final String REP_SYSTEM_USER = "rep:SystemUser";
private static final String REP_USER = "rep:User";
@@ -52,16 +53,17 @@
* @param path - the original repository path of the user in the content-package.
* @param intermediatePath - the intermediate path the user should have - most likely the (direct) parent of the path.
*/
- public SystemUserParser(@NotNull ContentPackage2FeatureModelConverter converter, @NotNull RepoPath path, @NotNull RepoPath intermediatePath) {
+ public UserParser(@NotNull ContentPackage2FeatureModelConverter converter, @NotNull RepoPath path, @NotNull RepoPath intermediatePath) {
super(converter, path, intermediatePath, REP_SYSTEM_USER, REP_USER);
}
@Override
- void handleUser(@NotNull String id) {
+ void handleUser(@NotNull String id, @NotNull Attributes attributes) {
+ String disabledReason = attributes.getValue("rep:disabled");
if (REP_USER.equals(detectedPrimaryType)) {
- converter.getAclManager().addUser(new User(id, path, intermediatePath));
- } else{
- converter.getAclManager().addSystemUser(new SystemUser(id, path, intermediatePath));
+ converter.getAclManager().addUser(new User(id, path, intermediatePath, disabledReason));
+ } else {
+ converter.getAclManager().addSystemUser(new SystemUser(id, path, intermediatePath, disabledReason));
}
}
}
diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java
index 40c54dc..96b8fc9 100644
--- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java
+++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java
@@ -30,7 +30,9 @@
import org.junit.Test;
import java.io.StringReader;
+import java.lang.reflect.Field;
import java.util.List;
+import java.util.Set;
import static org.apache.sling.feature.cpconverter.Util.normalize;
import static org.junit.Assert.assertEquals;
@@ -43,6 +45,7 @@
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
public class UsersEntryHandlerTest {
@@ -88,6 +91,24 @@
}
@Test
+ public void parseDisabledSystemUser() throws Exception {
+ String path = "/jcr_root/home/users/system/services/random1/.content.xml";
+ Extension repoinitExtension = parseAndSetRepoinit(path);
+
+ assertNotNull(repoinitExtension);
+ assertEquals(ExtensionType.TEXT, repoinitExtension.getType());
+ assertTrue(repoinitExtension.isRequired());
+
+ String expected = normalize("create service user service1 with path system/services\ndisable service user service1 : \"a reason\"\n");
+ String actual = repoinitExtension.getText();
+ assertEquals(expected, actual);
+
+ RepoInitParser repoInitParser = new RepoInitParserService();
+ List<Operation> operations = repoInitParser.parse(new StringReader(actual));
+ assertFalse(operations.isEmpty());
+ }
+
+ @Test
public void unrecognisedSystemUserJcrNode() throws Exception {
String path = "/jcr_root/home/users/system/asd-share-commons/asd-index-definition-invalid/.content.xml";
Extension repoinitExtension = parseAndSetRepoinit(path);
@@ -120,6 +141,22 @@
}
@Test
+ public void testDisabledUser() throws Exception {
+ String path = "/jcr_root/home/users/a/disableduser/.content.xml";
+ DefaultAclManager aclManager = new DefaultAclManager();
+ TestUtils.createRepoInitExtension(usersEntryHandler, aclManager, path, getClass().getResourceAsStream(path.substring(1)));
+
+ Field f = DefaultAclManager.class.getDeclaredField("users");
+ f.setAccessible(true);
+
+ Set<User> users = (Set<User>) f.get(aclManager);
+ assertNotNull(users);
+ assertEquals(1, users.size());
+ String disabledReason = users.iterator().next().getDisabledReason();
+ assertEquals("a reason", disabledReason);
+ }
+
+ @Test
public void parseUserWithConfig() throws Exception {
String path = "/jcr_root/rep:security/rep:authorizables/rep:users/a/author/.content.xml";
String filePath = path.substring(1).replace("rep:", "_rep_");
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/a/disableduser/.content.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/a/disableduser/.content.xml
new file mode 100644
index 0000000..c229ef3
--- /dev/null
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/a/disableduser/.content.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with this
+ work for additional information regarding copyright ownership. The ASF
+ licenses this file to You under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ License for the specific language governing permissions and limitations under
+ the License.
+-->
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
+ jcr:mixinTypes="[rep:AccessControllable]"
+ jcr:primaryType="rep:User"
+ jcr:uuid="098f6bcd-4621-3373-8ade-4e832627b4f6"
+ rep:authorizableId="test"
+ rep:principalName="test"
+ rep:disabled="a reason"/>
\ No newline at end of file
diff --git a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random1/.content.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random1/.content.xml
index 5b113ba..cf9b705 100644
--- a/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random1/.content.xml
+++ b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random1/.content.xml
@@ -19,4 +19,5 @@
jcr:primaryType="rep:SystemUser"
jcr:uuid="0a39b2bb-8594-3d80-a4fe-3464cfb7038b"
rep:authorizableId="service1"
- rep:principalName="service1"/>
+ rep:principalName="service1"
+ rep:disabled="a reason"/>
