SLING-6419: Switch JcrSystemUserValidator to a service user preventing endless recursion using a thread local.
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidator.java b/src/main/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidator.java
index 4101e44..5080140 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidator.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidator.java
@@ -82,6 +82,16 @@
private boolean allowOnlySystemUsers;
+ /*
+ * We have to prevent a cycle if we are trying to login ourselves
+ */
+ private final ThreadLocal<Boolean> cycleDetection = new ThreadLocal<Boolean>() {
+ @Override
+ protected Boolean initialValue() {
+ return false;
+ }
+ };
+
public JcrSystemUserValidator() {
Method m = null;
try {
@@ -99,6 +109,9 @@
@Override
public boolean isValid(final String serviceUserId, final String serviceName, final String subServiceName) {
+ if (cycleDetection.get()) {
+ return true;
+ }
if (serviceUserId == null) {
log.debug("The provided service user id is null");
return false;
@@ -111,22 +124,20 @@
log.debug("The provided service user id '{}' has been already validated and is a known JCR system user id", serviceUserId);
return true;
} else {
- Session administrativeSession = null;
+ Session session = null;
try {
try {
/*
- * TODO: Instead of using the deprecated loginAdministrative
- * method, this bundle could be configured with an appropriate
- * user for service authentication and do:
- * tmpSession = repository.loginService(null, workspace);
- * For now, we keep loginAdministrative as switching to a service user
- * will result in a endless recursion (this method checks if
- * a service user is allowed, so using a service user here
- * calls this method again...and again...and again)
+ * We have to prevent a cycle if we are trying to login ourselves
*/
- administrativeSession = repository.loginAdministrative(null);
- if (administrativeSession instanceof JackrabbitSession) {
- final UserManager userManager = ((JackrabbitSession) administrativeSession).getUserManager();
+ cycleDetection.set(true);
+ try {
+ session = repository.loginService(null, null);
+ } finally {
+ cycleDetection.set(false);
+ }
+ if (session instanceof JackrabbitSession) {
+ final UserManager userManager = ((JackrabbitSession) session).getUserManager();
final Authorizable authorizable = userManager.getAuthorizable(serviceUserId);
if (isValidSystemUser(authorizable)) {
validIds.add(serviceUserId);
@@ -138,8 +149,8 @@
log.warn("Could not get user information", e);
}
} finally {
- if (administrativeSession != null) {
- administrativeSession.logout();
+ if (session != null) {
+ session.logout();
}
}
log.warn("The provided service user id '{}' is not a known JCR system user id and therefore not allowed in the Sling Service User Mapper.", serviceUserId);
@@ -149,6 +160,9 @@
@Override
public boolean isValid(Iterable<String> servicePrincipalNames, String serviceName, String subServiceName) {
+ if (cycleDetection.get()) {
+ return true;
+ }
if (servicePrincipalNames == null) {
log.debug("The provided service principal names are null");
return false;
@@ -157,7 +171,8 @@
log.debug("There is no enforcement of JCR system users, therefore service principal names '{}' are valid", servicePrincipalNames);
return true;
}
- Session administrativeSession = null;
+
+ Session session = null;
UserManager userManager = null;
Set<String> invalid = new HashSet<>();
try {
@@ -165,20 +180,18 @@
if (validPrincipalNames.contains(pName)) {
log.debug("The provided service principal name '{}' has been already validated and is a known JCR system user", pName);
} else {
- /*
- * TODO: Instead of using the deprecated loginAdministrative
- * method, this bundle could be configured with an appropriate
- * user for service authentication and do:
- * tmpSession = repository.loginService(null, workspace);
- * For now, we keep loginAdministrative as switching to a service user
- * will result in a endless recursion (this method checks if
- * a sservice user is allowed, so using a service user here
- * calls this method again...and again...and again)
- */
- if (administrativeSession == null) {
- administrativeSession = repository.loginAdministrative(null);
- if (administrativeSession instanceof JackrabbitSession) {
- userManager = ((JackrabbitSession) administrativeSession).getUserManager();
+ if (session == null) {
+ /*
+ * We have to prevent a cycle if we are trying to login ourselves
+ */
+ cycleDetection.set(true);
+ try {
+ session = repository.loginService(null, null);
+ } finally {
+ cycleDetection.set(false);
+ }
+ if (session instanceof JackrabbitSession) {
+ userManager = ((JackrabbitSession) session).getUserManager();
} else {
log.debug("Unable to validate service user principals, JackrabbitSession expected.");
return false;
@@ -203,8 +216,8 @@
} catch (final RepositoryException e) {
log.warn("Could not get user information", e);
} finally {
- if (administrativeSession != null) {
- administrativeSession.logout();
+ if (session != null) {
+ session.logout();
}
}
return invalid.isEmpty();
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidatorTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidatorTest.java
index 929d431..ae36024 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidatorTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/JcrSystemUserValidatorTest.java
@@ -19,10 +19,16 @@
import java.lang.reflect.Field;
import java.util.Collections;
+import javax.jcr.Credentials;
+import javax.jcr.LoginException;
+import javax.jcr.NoSuchWorkspaceException;
import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
import javax.naming.NamingException;
import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
+import org.apache.sling.jcr.api.SlingRepository;
import org.junit.Before;
import org.junit.Test;
@@ -38,7 +44,75 @@
jcrSystemUserValidator = new JcrSystemUserValidator();
Field repositoryField = jcrSystemUserValidator.getClass().getDeclaredField("repository");
repositoryField.setAccessible(true);
- repositoryField.set(jcrSystemUserValidator, getRepository());
+ final SlingRepository delegate = getRepository();
+
+ SlingRepository repository = new SlingRepository() {
+ @Override
+ public String getDefaultWorkspace() {
+ return delegate.getDefaultWorkspace();
+ }
+
+ @Override
+ public Session loginAdministrative(String s) throws LoginException, RepositoryException {
+ return delegate.loginAdministrative(s);
+ }
+
+ @Override
+ public Session loginService(String s, String s1) throws LoginException, RepositoryException {
+ return delegate.loginAdministrative(s1);
+ }
+
+ @Override
+ public String[] getDescriptorKeys() {
+ return delegate.getDescriptorKeys();
+ }
+
+ @Override
+ public boolean isStandardDescriptor(String s) {
+ return delegate.isStandardDescriptor(s);
+ }
+
+ @Override
+ public boolean isSingleValueDescriptor(String s) {
+ return delegate.isSingleValueDescriptor(s);
+ }
+
+ @Override
+ public Value getDescriptorValue(String s) {
+ return delegate.getDescriptorValue(s);
+ }
+
+ @Override
+ public Value[] getDescriptorValues(String s) {
+ return delegate.getDescriptorValues(s);
+ }
+
+ @Override
+ public String getDescriptor(String s) {
+ return delegate.getDescriptor(s);
+ }
+
+ @Override
+ public Session login(Credentials credentials, String s) throws LoginException, NoSuchWorkspaceException, RepositoryException {
+ return delegate.login(credentials, s);
+ }
+
+ @Override
+ public Session login(Credentials credentials) throws LoginException, RepositoryException {
+ return delegate.login(credentials);
+ }
+
+ @Override
+ public Session login(String s) throws LoginException, NoSuchWorkspaceException, RepositoryException {
+ return delegate.login(s);
+ }
+
+ @Override
+ public Session login() throws LoginException, RepositoryException {
+ return delegate.login();
+ }
+ };
+ repositoryField.set(jcrSystemUserValidator, repository);
}
@Test
