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 <[email protected]> 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"/>
