You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by an...@apache.org on 2021/04/13 12:24:17 UTC
[sling-org-apache-sling-feature-cpconverter] branch SLING-10286
created (now 82b2835)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch SLING-10286
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git.
at 82b2835 SLING-10286 : Converter ignores disabled status of system user
This branch includes the following new commits:
new 82b2835 SLING-10286 : Converter ignores disabled status of system user
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-feature-cpconverter] 01/01: SLING-10286 :
Converter ignores disabled status of system user
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch SLING-10286
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit 82b2835fca5efe5a4681c6bdbf0c108da7f72241
Author: angela <an...@adobe.com>
AuthorDate: Tue Apr 13 14:23:22 2021 +0200
SLING-10286 : Converter ignores disabled status of system user
---
.../cpconverter/accesscontrol/AbstractUser.java | 12 +++++++
.../accesscontrol/DefaultAclManager.java | 5 +++
.../cpconverter/accesscontrol/SystemUser.java | 11 +++++++
.../feature/cpconverter/accesscontrol/User.java | 11 +++++++
.../cpconverter/handlers/AbstractUserParser.java | 4 +--
.../cpconverter/handlers/GroupEntryHandler.java | 3 +-
.../cpconverter/handlers/UsersEntryHandler.java | 16 ++++++----
.../handlers/UsersEntryHandlerTest.java | 37 ++++++++++++++++++++++
.../random1 => a/disableduser}/.content.xml | 10 +++---
.../users/system/services/random1/.content.xml | 3 +-
10 files changed, 97 insertions(+), 15 deletions(-)
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 @@ package org.apache.sling.feature.cpconverter.accesscontrol;
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 @@ abstract class AbstractUser {
private final RepoPath path;
private final RepoPath intermediatePath;
+
+ private final String disabledReason;
/**
* @param id - the authorizableId to use.
@@ -34,9 +37,14 @@ abstract class AbstractUser {
* @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 @@ abstract class AbstractUser {
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.feature.cpconverter.vltpkg.VaultPackageAssembler;
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 @@ public class DefaultAclManager implements AclManager, EnforceInfo {
// 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 @@ package org.apache.sling.feature.cpconverter.accesscontrol;
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 class SystemUser extends AbstractUser {
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 @@ package org.apache.sling.feature.cpconverter.accesscontrol;
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 class User extends AbstractUser {
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 @@ abstract class AbstractUserParser extends AbstractJcrNodeParser<Void> {
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 @@ abstract class AbstractUserParser extends AbstractJcrNodeParser<Void> {
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.ContentPackage2FeatureModelConverter
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 @@ public final class GroupEntryHandler extends AbstractUserEntryHandler {
}
@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.SystemUser;
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 @@ public final class UsersEntryHandler extends AbstractUserEntryHandler {
@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 @@ public final class UsersEntryHandler extends AbstractUserEntryHandler {
* @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.Before;
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.mock;
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 @@ public class UsersEntryHandlerTest {
}
@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 @@ public class UsersEntryHandlerTest {
}
@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/system/services/random1/.content.xml b/src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/a/disableduser/.content.xml
similarity index 77%
copy from src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/system/services/random1/.content.xml
copy to src/test/resources/org/apache/sling/feature/cpconverter/handlers/jcr_root/home/users/a/disableduser/.content.xml
index 5b113ba..c229ef3 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/a/disableduser/.content.xml
@@ -16,7 +16,9 @@
the License.
-->
<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rep="internal"
- jcr:primaryType="rep:SystemUser"
- jcr:uuid="0a39b2bb-8594-3d80-a4fe-3464cfb7038b"
- rep:authorizableId="service1"
- rep:principalName="service1"/>
+ 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"/>