This is an automated email from the ASF dual-hosted git repository. pauls pushed a commit to branch SLING-6419 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit 40a601e2cd2cc176c26dcfd96877efd200170c8d Author: Karl Pauls <[email protected]> AuthorDate: Tue Nov 14 17:30:02 2017 +0100 SLING-6419: Switch JcrSystemUserValidator to a service user preventing endless recursion using a thread local. --- .../resource/internal/JcrSystemUserValidator.java | 75 ++++++++++++--------- .../internal/JcrSystemUserValidatorTest.java | 76 +++++++++++++++++++++- 2 files changed, 119 insertions(+), 32 deletions(-) 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin @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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin @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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin 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 @@ public class JcrSystemUserValidator implements ServiceUserValidator, ServicePrin } 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 @@ package org.apache.sling.jcr.resource.internal; 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 @@ public class JcrSystemUserValidatorTest extends RepositoryTestBase { 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 -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
