Revert "SLING-7805 - NPE in Oak SessionImpl when starting up"
Restore the fix for SLING-3524
diff --git a/pom.xml b/pom.xml
index 6a44844..fea1412 100644
--- a/pom.xml
+++ b/pom.xml
@@ -54,6 +54,7 @@
<plugin>
<groupId>org.apache.sling</groupId>
<artifactId>maven-sling-plugin</artifactId>
+ <version>2.3.2</version>
<executions>
<execution>
<id>generate-adapter-metadata</id>
@@ -176,7 +177,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>2.16.4</version>
+ <version>2.18.2</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
index 8cc6a55..064f095 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrProviderStateFactory.java
@@ -144,7 +144,12 @@
@NotNull final Map<String, Object> authenticationInfo,
@Nullable final BundleContext ctx
) throws LoginException {
- final Session impersonatedSession = handleImpersonation(session, authenticationInfo, logoutSession);
+ boolean explicitSessionUsed = (getSession(authenticationInfo) != null);
+ final Session impersonatedSession = handleImpersonation(session, authenticationInfo, logoutSession, explicitSessionUsed);
+ if (impersonatedSession != session && explicitSessionUsed) {
+ // update the session in the auth info map in case the resolver gets cloned in the future
+ authenticationInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, impersonatedSession);
+ }
// if we're actually impersonating, we're responsible for closing the session we've created, regardless
// of what the original logoutSession value was.
boolean doLogoutSession = logoutSession || (impersonatedSession != session);
@@ -166,25 +171,35 @@
* @param logoutSession
* whether to logout the <code>session</code> after impersonation
* or not.
+ * @param explicitSessionUsed
+ * whether the JCR session was explicitly given in the auth info or not.
* @return The original session or impersonated session.
* @throws LoginException
* If something goes wrong.
*/
private static Session handleImpersonation(final Session session, final Map<String, Object> authenticationInfo,
- final boolean logoutSession) throws LoginException {
+ final boolean logoutSession, boolean explicitSessionUsed) throws LoginException {
final String sudoUser = getSudoUser(authenticationInfo);
- if (sudoUser != null && !session.getUserID().equals(sudoUser)) {
- try {
- final SimpleCredentials creds = new SimpleCredentials(sudoUser, new char[0]);
+ // Do we need session.impersonate() because we are asked to impersonate another user?
+ boolean needsSudo = (sudoUser != null) && !session.getUserID().equals(sudoUser);
+ // Do we need session.impersonate() to get an independent copy of the session we were given in the auth info?
+ boolean needsCloning = !needsSudo && explicitSessionUsed && authenticationInfo.containsKey(ResourceProvider.AUTH_CLONE);
+ try {
+ if (needsSudo) {
+ SimpleCredentials creds = new SimpleCredentials(sudoUser, new char[0]);
copyAttributes(creds, authenticationInfo);
creds.setAttribute(ResourceResolver.USER_IMPERSONATOR, session.getUserID());
return session.impersonate(creds);
- } catch (final RepositoryException re) {
- throw getLoginException(re);
- } finally {
- if (logoutSession) {
- session.logout();
- }
+ } else if (needsCloning) {
+ SimpleCredentials creds = new SimpleCredentials(session.getUserID(), new char[0]);
+ copyAttributes(creds, authenticationInfo);
+ return session.impersonate(creds);
+ }
+ } catch (final RepositoryException re) {
+ throw getLoginException(re);
+ } finally {
+ if (logoutSession) {
+ session.logout();
}
}
return session;
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java
new file mode 100644
index 0000000..a471446
--- /dev/null
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderSessionHandlingTest.java
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.jcr.resource.internal.helper.jcr;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeThat;
+import static org.junit.Assume.assumeTrue;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.jcr.Session;
+
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.commons.testing.jcr.RepositoryProvider;
+import org.apache.sling.jcr.api.SlingRepository;
+import org.apache.sling.jcr.resource.api.JcrResourceConstants;
+import org.apache.sling.spi.resource.provider.ResolveContext;
+import org.apache.sling.spi.resource.provider.ResourceProvider;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.mockito.Mockito;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.component.ComponentContext;
+
+@RunWith(Parameterized.class)
+public class JcrResourceProviderSessionHandlingTest {
+
+ private enum LoginStyle {USER, SESSION};
+
+ private static final String AUTH_USER = "admin";
+ private static final char[] AUTH_PASSWORD = "admin".toCharArray();
+ private static final String SUDO_USER = "anonymous";
+
+ @Parameters(name = "loginStyle= {0}, sudo = {1}, clone = {2}")
+ public static List<Object[]> data() {
+
+ LoginStyle[] loginStyles = LoginStyle.values();
+ boolean[] sudoOptions = new boolean[] {false, true};
+ boolean[] cloneOptions = new boolean[] {false, true};
+
+ // Generate all possible combinations into data.
+ List<Object[]> data = new ArrayList<>();
+ Object[] dataPoint = new Object[3];
+ for (LoginStyle loginStyle : loginStyles) {
+ dataPoint[0] = loginStyle;
+ for (boolean sudo : sudoOptions) {
+ dataPoint[1] = sudo;
+ for (boolean clone : cloneOptions) {
+ dataPoint[2] = clone;
+ data.add(dataPoint.clone());
+ }
+ }
+ }
+ return data;
+ }
+
+ @Parameter(0)
+ public LoginStyle loginStyle;
+
+ @Parameter(1)
+ public boolean useSudo;
+
+ @Parameter(2)
+ public boolean doClone;
+
+ // Session we're using when loginStyle == SESSION, null otherwise.
+ private Session explicitSession;
+
+ private JcrResourceProvider jcrResourceProvider;
+ private JcrProviderState jcrProviderState;
+
+ @Before
+ public void setUp() throws Exception {
+ SlingRepository repo = RepositoryProvider.instance().getRepository();
+ Map<String, Object> authInfo = new HashMap<>();
+ switch (loginStyle) {
+ case USER:
+ authInfo.put(ResourceResolverFactory.USER, AUTH_USER);
+ authInfo.put(ResourceResolverFactory.PASSWORD, AUTH_PASSWORD);
+ break;
+ case SESSION:
+ explicitSession = repo.loginAdministrative(null);
+ authInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, explicitSession);
+ break;
+ }
+
+ if (useSudo) {
+ authInfo.put(ResourceResolverFactory.USER_IMPERSONATION, SUDO_USER);
+ }
+
+ if (doClone) {
+ authInfo.put(ResourceProvider.AUTH_CLONE, true);
+ }
+
+ ComponentContext ctx = mock(ComponentContext.class);
+ when(ctx.locateService(anyString(), Mockito.<ServiceReference<Object>>any())).thenReturn(repo);
+
+ jcrResourceProvider = new JcrResourceProvider();
+ jcrResourceProvider.activate(ctx);
+
+ jcrProviderState = jcrResourceProvider.authenticate(authInfo);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+
+ // Some tests do a logout, so check for liveness before trying to log out.
+ if (jcrProviderState.getSession().isLive()) {
+ jcrResourceProvider.logout(jcrProviderState);
+ }
+
+ jcrResourceProvider.deactivate();
+
+ if (explicitSession != null) {
+ explicitSession.logout();
+ }
+ }
+
+ @Test
+ public void sessionUsesCorrectUser() {
+ String expectedUser = useSudo ? SUDO_USER : AUTH_USER;
+ assertEquals(expectedUser, jcrProviderState.getSession().getUserID());
+ }
+
+ @Test
+ public void explicitSessionNotClosedOnLogout() {
+ assumeTrue(loginStyle == LoginStyle.SESSION);
+
+ jcrResourceProvider.logout(jcrProviderState);
+
+ assertTrue(explicitSession.isLive());
+ }
+
+ @Test
+ public void sessionsDoNotLeak() {
+ // This test is only valid if we either didn't pass an explicit session,
+ // or the provider had to clone it. Sessions created by the provider
+ // must be closed by the provider, or we have a session leak.
+ assumeThat(jcrProviderState.getSession(), is(not(sameInstance(explicitSession))));
+
+ jcrResourceProvider.logout(jcrProviderState);
+
+ assertFalse(jcrProviderState.getSession().isLive());
+ }
+
+ @Test
+ public void impersonatorIsReportedCorrectly() {
+ assumeTrue(useSudo);
+
+ @SuppressWarnings("unchecked")
+ ResolveContext<JcrProviderState> mockContext = mock(ResolveContext.class);
+ when(mockContext.getProviderState()).thenReturn(jcrProviderState);
+ Object reportedImpersonator = jcrResourceProvider.getAttribute(mockContext, ResourceResolver.USER_IMPERSONATOR);
+
+ assertEquals(AUTH_USER, reportedImpersonator);
+ }
+
+ @Test
+ public void clonesAreIndependent() {
+ assumeTrue(loginStyle == LoginStyle.SESSION && doClone);
+
+ assertNotSame(explicitSession, jcrProviderState.getSession());
+ }
+
+}
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
index aa40162..32c6bfb 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProviderTest.java
@@ -19,18 +19,11 @@
package org.apache.sling.jcr.resource.internal.helper.jcr;
import java.security.Principal;
-import java.util.HashMap;
-import java.util.Map;
import javax.jcr.Repository;
-import javax.jcr.RepositoryException;
import javax.jcr.Session;
-import javax.naming.NamingException;
-import org.apache.sling.api.resource.LoginException;
-import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.commons.testing.jcr.RepositoryTestBase;
-import org.apache.sling.jcr.resource.api.JcrResourceConstants;
import org.apache.sling.spi.resource.provider.ResolveContext;
import org.junit.Assert;
import org.mockito.Mockito;
@@ -47,33 +40,23 @@
super.setUp();
// create the session
session = getSession();
- }
-
- @Override
- protected void tearDown() throws Exception {
- super.tearDown();
- }
-
- public void testAdaptTo_Principal() {
- jcrResourceProvider = new JcrResourceProvider();
- ResolveContext ctx = Mockito.mock(ResolveContext.class);
- Mockito.when(ctx.getProviderState()).thenReturn(new JcrProviderState(session, null, false));
- Assert.assertNotNull(jcrResourceProvider.adaptTo(ctx, Principal.class));
- }
-
- public void testLeakOnSudo() throws LoginException, RepositoryException, NamingException {
Repository repo = getRepository();
ComponentContext ctx = Mockito.mock(ComponentContext.class);
Mockito.when(ctx.locateService(Mockito.anyString(), Mockito.any(ServiceReference.class))).thenReturn(repo);
jcrResourceProvider = new JcrResourceProvider();
jcrResourceProvider.activate(ctx);
- Map<String, Object> authInfo = new HashMap<String, Object>();
- authInfo.put(JcrResourceConstants.AUTHENTICATION_INFO_SESSION, session);
- authInfo.put(ResourceResolverFactory.USER_IMPERSONATION, "anonymous");
- JcrProviderState providerState = jcrResourceProvider.authenticate(authInfo);
- Assert.assertNotEquals("Impersonation didn't start new session", session, providerState.getSession());
- jcrResourceProvider.logout(providerState);
- assertFalse("Impersonated session wasn't closed.", providerState.getSession().isLive());
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ jcrResourceProvider.deactivate();
+ super.tearDown();
+ }
+
+ public void testAdaptTo_Principal() {
+ ResolveContext ctx = Mockito.mock(ResolveContext.class);
+ Mockito.when(ctx.getProviderState()).thenReturn(new JcrProviderState(session, null, false));
+ Assert.assertNotNull(jcrResourceProvider.adaptTo(ctx, Principal.class));
}
}
