Skip to content

Commit

Permalink
SLING-12329 - Backwards compatibility for legacy repoinit statement r…
Browse files Browse the repository at this point in the history 
…eordering (#54)
  • Loading branch information
@jsedding
jsedding committed May 28, 2024
1 parent 6819aaf commit 01b4df0
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 40 deletions.
24 changes: 12 additions & 12 deletions src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ private static void setAcl(Session session, List<String> principals, String jcrP

final String [] privArray = privileges.toArray(new String[privileges.size()]);
final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(acMgr, privArray);

JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
checkState(acl != null, "No JackrabbitAccessControlList available for path {0}", jcrPath);

Expand Down Expand Up @@ -197,7 +197,7 @@ private static Principal getPrincipal(Session session, String name, boolean igno

/**
* Remove resource-based access control setup for the principal with the given name.
*
*
* @param session
* @param principalName
* @throws RepositoryException
Expand All @@ -210,7 +210,7 @@ public static void removePolicy(@NotNull Session session, @NotNull final String
// in case import-behavior is configured to be ABORT.
principal = new PrincipalImpl(principalName);
}

JackrabbitAccessControlManager acMgr = getJACM(session);
for (JackrabbitAccessControlPolicy policy : acMgr.getPolicies(principal)) {
// make sure not to remove the principal-based access control list but instead only drop
Expand All @@ -223,7 +223,7 @@ public static void removePolicy(@NotNull Session session, @NotNull final String

/**
* Remove resource-based access control setup defined for the specified paths.
*
*
* @param session
* @param paths
* @throws RepositoryException
Expand All @@ -244,7 +244,7 @@ public static void removePolicies(@NotNull Session session, @NotNull List<String
}
}
}

public static void removeEntries(@NotNull Session session, @NotNull List<String> principals, @NotNull List<String> paths) throws RepositoryException {
Set<String> principalNames = new HashSet<>(principals);
AccessControlManager acMgr = session.getAccessControlManager();
Expand Down Expand Up @@ -284,7 +284,7 @@ public static void removeEntries(@NotNull Session session, @NotNull List<String>

LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, session);
Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privileges.toArray(new String[0]));

for (AccessControlEntry ace : acl.getAccessControlEntries()) {
Principal principal = ace.getPrincipal();
if (!principalNames.contains(principal.getName())) {
Expand Down Expand Up @@ -407,7 +407,7 @@ public static void removePrincipalEntries(Session session, String principalName,
}

/**
* Remove principal-based access control setup for the principal with the given name.
* Remove principal-based access control setup for the principal with the given name.
*
* @param session
* @param principalName
Expand All @@ -428,13 +428,13 @@ public static void removePrincipalPolicy(@NotNull Session session, @NotNull Stri
acMgr.removePolicy(acl.getPath(), acl);
}
}

private static boolean isValidPath(@NotNull Session session, @Nullable String jcrPath) throws RepositoryException {
return jcrPath == null || session.nodeExists(jcrPath);
}

/**
*
*
* @param acMgr the access control manager
* @param principal the principal
* @return the first available {@link PrincipalAccessControlList} bound to the given principal or {@code null} of <a href="https://jackrabbit.apache.org/oak/docs/security/authorization/principalbased.html">principal-based authorization</a> is not enabled for the given principal
Expand All @@ -456,7 +456,7 @@ private static JackrabbitAccessControlList getAccessControlList(@NotNull AccessC
}

@Nullable
private static PrincipalAccessControlList getPrincipalAccessControlList(@NotNull JackrabbitAccessControlManager acMgr,
private static PrincipalAccessControlList getPrincipalAccessControlList(@NotNull JackrabbitAccessControlManager acMgr,
@NotNull Principal principal, boolean includeApplicable) throws RepositoryException {
PrincipalAccessControlList acl = null;
for (JackrabbitAccessControlPolicy policy : acMgr.getPolicies(principal)) {
Expand Down Expand Up @@ -567,7 +567,7 @@ private static String toString(JackrabbitAccessControlEntry entry) throws Reposi

private static void checkState(boolean expression, String msgPattern, Object... args) {
if (!expression) {
if (args != null) {
if (args == null) {
throw new IllegalStateException(msgPattern);
} else {
throw new IllegalStateException(MessageFormat.format(msgPattern, args));
Expand Down Expand Up @@ -635,7 +635,7 @@ public boolean isEqual(AccessControlEntry other) {
throw new IllegalStateException("Cannot verify equivalence of access control entries", e);
}
}

private Set<Privilege> expandPrivileges(Privilege[] privileges){
Set<Privilege> expandedSet = new HashSet<>();

Expand Down
84 changes: 67 additions & 17 deletions src/main/java/org/apache/sling/jcr/repoinit/impl/JcrRepoInitOpsProcessorImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@
package org.apache.sling.jcr.repoinit.impl;

import java.util.List;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Stream;

import javax.jcr.Session;

import org.apache.sling.jcr.repoinit.JcrRepoInitOpsProcessor;
import org.apache.sling.repoinit.parser.operations.Operation;
import org.apache.sling.repoinit.parser.operations.OperationVisitor;
import org.osgi.framework.Constants;
import org.osgi.service.component.annotations.Component;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static java.util.Arrays.asList;
import static java.util.Collections.singleton;
Expand All @@ -38,28 +43,73 @@
})
public class JcrRepoInitOpsProcessorImpl implements JcrRepoInitOpsProcessor {

private static final Logger log = LoggerFactory.getLogger(JcrRepoInitOpsProcessorImpl.class);

/**
* Apply the supplied operations: first the namespaces and nodetypes
* registrations, then the service users, paths and ACLs.
*/
@Override
public void apply(Session session, List<Operation> ops) {
Stream.of(
// register namespaces first
singleton(new NamespacesVisitor(session)),
// then create node types and privileges, both use namespaces
asList(
new NodetypesVisitor(session),
new PrivilegeVisitor(session)),
// finally apply everything else
asList(
new UserVisitor(session),
new NodeVisitor(session),
new AclVisitor(session),
new GroupMembershipVisitor(session),
new NodePropertiesVisitor(session))
).forEach(visitorGroup -> {
ops.forEach(op -> visitorGroup.forEach(op::accept));
});
AtomicReference<Operation> lastAttemptedOperation = new AtomicReference<>();
try {
Stream.of(
// register namespaces first
singleton(new NamespacesVisitor(session)),
// then create node types and privileges, both use namespaces
asList(
new NodetypesVisitor(session),
new PrivilegeVisitor(session)),
// finally apply everything else
asList(
new UserVisitor(session),
new NodeVisitor(session),
new AclVisitor(session),
new GroupMembershipVisitor(session),
new NodePropertiesVisitor(session))
).forEach(visitorGroup -> {
ops.forEach(op -> {
lastAttemptedOperation.set(op);
visitorGroup.forEach(op::accept);
});
});
} catch (RepoInitException originalFailure) {
handleLegacyOrderingSupport(session, ops, originalFailure, lastAttemptedOperation);
}
}

// support legacy statement reordering for backwards compatibility
private static void handleLegacyOrderingSupport(Session session, List<Operation> ops, RepoInitException originalFailure, AtomicReference<Operation> lastAttemptedOperation) {
try {
session.refresh(false); // drop transient changes

final OperationVisitor[] visitors = {
new NamespacesVisitor(session),
new NodetypesVisitor(session),
new PrivilegeVisitor(session),
new UserVisitor(session),
new NodeVisitor(session),
new AclVisitor(session),
new GroupMembershipVisitor(session),
new NodePropertiesVisitor(session)
};

for (OperationVisitor v : visitors) {
for (Operation op : ops) {
op.accept(v);
}
}

log.warn("DEPRECATION - The repoinit script being executed relies on a bug causing repoinit statements " +
"to be reordered (SLING-12107). For now your repoinit script was applied successfully in legacy " +
"mode. Please review and fix the ordering of your repoinit statements to avoid future issues. " +
"The code supporting the legacy order will be removed in a future release. The new code " +
"failed on the statement \"{}\". The original exception message was: {}",
Optional.ofNullable(lastAttemptedOperation.get()).map(Operation::asRepoInitString).orElse("unknown"),
originalFailure.getMessage());
} catch (Exception legacyFailure) {
// rethrow the originalFailure if the legacy code also failed
throw originalFailure;
}
}
}
45 changes: 37 additions & 8 deletions src/test/java/org/apache/sling/jcr/repoinit/ExecutionOrderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,21 @@
*/
package org.apache.sling.jcr.repoinit;

import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import ch.qos.logback.classic.Level;
import org.apache.sling.jcr.repoinit.impl.TestUtil;
import org.apache.sling.jcr.repoinit.impl.UserUtil;
import org.apache.sling.repoinit.parser.RepoInitParsingException;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.apache.sling.testing.mock.sling.ResourceResolverType;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
import org.apache.sling.testing.mock.sling.junit.SlingContextBuilder;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.Value;
import java.util.UUID;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

/** Verify that namespaces and nodetypes are executed before path creation. But otherwise,
* the execution order should match the order of the statements.
Expand All @@ -43,9 +39,9 @@ public class ExecutionOrderTest {

@Rule
public final SlingContext context = new SlingContext(ResourceResolverType.JCR_OAK);

private TestUtil U;

private static final String TEST_ID = UUID.randomUUID().toString();
private static final String NS_PREFIX = ExecutionOrderTest.class.getSimpleName();
private static final String NS_URI = "uri:" + NS_PREFIX + ":" + TEST_ID;
Expand Down Expand Up @@ -89,4 +85,37 @@ public void createGroupWithACLsThenDeleteGroup() throws RepositoryException, Rep
U.assertPrivileges(GROUP_NAME, PATH, true, "jcr:read");
U.assertPrivileges(GROUP_NAME, PATH, false, "jcr:write");
}

@Test
public void legacySetAclBeforeCreatingGroupTest() throws Exception {
try (LogCapture capture = new LogCapture("org.apache.sling.jcr.repoinit.impl.JcrRepoInitOpsProcessorImpl", true)) {
U.parseAndExecute(
"create path (nt:folder) " + PATH,
"set ACL for " + GROUP_NAME,
" allow jcr:read on " + PATH,
" deny jcr:write on " + PATH,
"end",
"create group " + GROUP_NAME
);

capture.assertContains(Level.WARN, "DEPRECATION", "set ACL for " + GROUP_NAME);
} catch (RuntimeException e) {
fail("Legacy support should have reordered \"create group\" before \"set ACL\"");
}
}

@Test
public void legacyAddMemberBeforeCreatingGroupTest() throws Exception {
try (LogCapture capture = new LogCapture("org.apache.sling.jcr.repoinit.impl.JcrRepoInitOpsProcessorImpl", true)) {
U.parseAndExecute(
"create group " + GROUP_NAME + "_1",
"add " + GROUP_NAME + "_1 to group " + GROUP_NAME,
"create group " + GROUP_NAME // reordered by legacy code
);

capture.assertContains(Level.WARN, "DEPRECATION", "add " + GROUP_NAME + "_1 to group ");
} catch (RuntimeException e) {
fail("Legacy support should have reordered \"create group\" before \"add ... to group\"");
}
}
}
6 changes: 3 additions & 3 deletions src/test/java/org/apache/sling/jcr/repoinit/LogCapture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;

/**
* Capture logs for testing
*
/**
* Capture logs for testing
*
* Initially cloned from: https://github.com/apache/sling-org-apache-sling-graphql-core/blob/0b1c1dd72ed04324ea84d2227c3223ec65b0b21e/src/test/java/org/apache/sling/graphql/core/util/LogCapture.java
*/
class LogCapture extends ListAppender<ILoggingEvent> implements AutoCloseable {
Expand Down

0 comments on commit 01b4df0

Please sign in to comment.