This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git
The following commit(s) were added to refs/heads/master by this push:
new 484b007 SLING-10267 : Authentication requirements from
SlingAuthenticator config registered twice
484b007 is described below
commit 484b0079e5b7cc2ab7665072578d9cdfdf9d0d39
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Wed Mar 31 07:24:55 2021 +0200
SLING-10267 : Authentication requirements from SlingAuthenticator config
registered twice
---
.../impl/AuthenticationRequirementsManager.java | 20 ++++-
.../AuthenticationRequirementsManagerTest.java | 97 +++++++++++++++++++---
.../auth/core/impl/SlingAuthenticatorTest.java | 13 ++-
3 files changed, 114 insertions(+), 16 deletions(-)
diff --git
a/src/main/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManager.java
b/src/main/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManager.java
index 4ed10d7..52eccda 100644
---
a/src/main/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManager.java
+++
b/src/main/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManager.java
@@ -36,6 +36,7 @@ import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.api.resource.mapping.ResourceMapper;
import org.apache.sling.auth.core.AuthConstants;
import org.osgi.framework.AllServiceListener;
+import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.InvalidSyntaxException;
@@ -97,6 +98,9 @@ public class AuthenticationRequirementsManager
/** Flag to indicate whether processing queue is running */
private final AtomicBoolean backgroundJobRunning = new
AtomicBoolean(false);
+ /** Own bundle id */
+ private final long bundleId;
+
/**
* Create a new manager
* @param executor For updating
@@ -119,6 +123,7 @@ public class AuthenticationRequirementsManager
final ResourceResolverFactory factory,
final SlingAuthenticator.Config config,
final Executor executor) {
+ this.bundleId = context.getBundle().getBundleId();
this.executor = executor;
this.resolverFactory = factory;
this.modified(config);
@@ -127,8 +132,11 @@ public class AuthenticationRequirementsManager
ServiceReference<?>[] refs = context.getAllServiceReferences(null,
FILTER_EXPR);
if (refs != null) {
for (final ServiceReference<?> ref : refs) {
- final Long id =
(Long)ref.getProperty(Constants.SERVICE_ID);
- this.queue(id, new Action(ActionType.ADDED, ref));
+ final Bundle bundle = ref.getBundle();
+ if ( bundle != null && bundle.getBundleId() !=
this.bundleId ) {
+ final Long id =
(Long)ref.getProperty(Constants.SERVICE_ID);
+ this.queue(id, new Action(ActionType.ADDED, ref));
+ }
}
}
@@ -184,10 +192,14 @@ public class AuthenticationRequirementsManager
*/
@Override
public void serviceChanged(final ServiceEvent event) {
+ final Bundle bundle = event.getServiceReference().getBundle();
+ if ( bundle != null && bundle.getBundleId() == this.bundleId ) {
+ // ignore all services from this bundle
+ return;
+ }
// modification of service properties, unregistration of the
// service or service properties does not contain requirements
- // property any longer (new event with type 8 added in OSGi Core
- // 4.2)
+ // property any longer (new event with type 8 added in OSGi Core 4.2)
final Long id =
(Long)event.getServiceReference().getProperty(Constants.SERVICE_ID);
if ((event.getType() & (ServiceEvent.UNREGISTERING |
ServiceEvent.MODIFIED_ENDMATCH)) != 0) {
queue(id, new Action(ActionType.REMOVED,
event.getServiceReference()));
diff --git
a/src/test/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManagerTest.java
b/src/test/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManagerTest.java
index 2cf1945..6082252 100644
---
a/src/test/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManagerTest.java
+++
b/src/test/java/org/apache/sling/auth/core/impl/AuthenticationRequirementsManagerTest.java
@@ -34,6 +34,7 @@ import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.api.resource.mapping.ResourceMapper;
import org.apache.sling.auth.core.AuthConstants;
import org.junit.Test;
+import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceEvent;
@@ -63,7 +64,7 @@ public class AuthenticationRequirementsManagerTest {
if ( !found ) {
int index = 0 ;
while ( !found && index < paths.length ) {
- if (paths[index].equals(h.path) &&
refs[index].equals(h.serviceReference) ) {
+ if (paths[index].equals(h.path) && ((refs[index] == null
&& h.serviceReference == null) || refs[index].equals(h.serviceReference)) ) {
found = true;
if ( requireAuth != null ) {
@@ -94,6 +95,10 @@ public class AuthenticationRequirementsManagerTest {
}
when(ref.compareTo(ref)).thenReturn(0);
+ final Bundle bundle = mock(Bundle.class);
+ when(bundle.getBundleId()).thenReturn(1L);
+ when(ref.getBundle()).thenReturn(bundle);
+
refs.add(ref);
return ref;
}
@@ -110,8 +115,18 @@ public class AuthenticationRequirementsManagerTest {
return factory;
}
- @Test public void testAddRemoveRegistration() throws LoginException {
+ private static final long BUNDLE_ID = 732;
+
+ private BundleContext createBundleContext() {
final BundleContext context = mock(BundleContext.class);
+ final Bundle bundle = mock(Bundle.class);
+ when(bundle.getBundleId()).thenReturn(BUNDLE_ID);
+ when(context.getBundle()).thenReturn(bundle);
+ return context;
+ }
+
+ @Test public void testAddRemoveRegistration() throws LoginException {
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
@@ -131,7 +146,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testAddUpdateRemoveRegistration() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path1a"));
when(mapper.getAllMappings("/path2")).thenReturn(Arrays.asList("/path2",
"/path2a"));
@@ -163,7 +178,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testDuplicateRegistration() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
when(mapper.getAllMappings("/path2")).thenReturn(Collections.singleton("/path2"));
@@ -189,7 +204,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testAddRemoveRegistrations() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
when(mapper.getAllMappings("/path2")).thenReturn(Collections.singleton("/path2"));
@@ -225,7 +240,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testModifyRegistration() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
when(mapper.getAllMappings("/path2")).thenReturn(Collections.singleton("/path2"));
@@ -253,7 +268,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testRegistrationWithMapping() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path2", "/path3"));
final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
@@ -271,7 +286,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testRegistrationAndUpdatingMapping() throws
LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path2", "/path3"));
final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
@@ -296,7 +311,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testAllowDeny() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(null),
SlingAuthenticatorTest.createDefaultConfig(), callable ->
callable.run());
@@ -310,7 +325,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testAllowDenyWithMapping() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path1a", "/path1b"));
@@ -338,7 +353,7 @@ public class AuthenticationRequirementsManagerTest {
}
@Test public void testSwitchAllowDeny() throws LoginException {
- final BundleContext context = mock(BundleContext.class);
+ final BundleContext context = createBundleContext();
final ResourceMapper mapper = mock(ResourceMapper.class);
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path1a"));
when(mapper.getAllMappings("/path2")).thenReturn(Arrays.asList("/path2",
"/path2a"));
@@ -367,4 +382,64 @@ public class AuthenticationRequirementsManagerTest {
assertEquals(3, manager.getHolders().size());
}
+
+ @Test public void testIgnoreRegistrationFromAuthCoreBundle() throws
LoginException {
+ final BundleContext context = createBundleContext();
+ final ResourceMapper mapper = mock(ResourceMapper.class);
+
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
+ final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
+ SlingAuthenticatorTest.createDefaultConfig(), callable ->
callable.run());
+
+ assertEquals(3, manager.getHolders().size());
+
+ final ServiceReference<?> ref = createServiceReference(new String[]
{"/path1"});
+ when(ref.getBundle().getBundleId()).thenReturn(BUNDLE_ID);
+ manager.serviceChanged(new ServiceEvent(ServiceEvent.REGISTERED, ref));
+
+ assertEquals(3, manager.getHolders().size());
+
+ manager.serviceChanged(new ServiceEvent(ServiceEvent.UNREGISTERING,
ref));
+
+ assertEquals(3, manager.getHolders().size());
+ }
+
+ @Test public void testInitialServices() throws Exception {
+ final BundleContext context = createBundleContext();
+
+ final ServiceReference<?> ref = createServiceReference(new String[]
{"/path1"});
+
+ when(context.getAllServiceReferences(null,
"(".concat(AuthConstants.AUTH_REQUIREMENTS).concat("=*)")))
+ .thenReturn(new ServiceReference[] {ref});
+
+ final ResourceMapper mapper = mock(ResourceMapper.class);
+
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
+ final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
+ SlingAuthenticatorTest.createDefaultConfig(), callable ->
callable.run());
+
+ assertPaths(manager, new String[] {"/path1"},
+ new ServiceReference<?>[] {ref});
+
+ manager.serviceChanged(new ServiceEvent(ServiceEvent.UNREGISTERING,
ref));
+
+ assertEquals(3, manager.getHolders().size());
+ }
+
+ @Test public void testInitialConfiguration() throws Exception {
+ final BundleContext context = createBundleContext();
+
+ final SlingAuthenticator.Config config =
SlingAuthenticatorTest.createDefaultConfig();
+ when(config.sling_auth_requirements()).thenReturn(new String[]
{"/path1", "-/path2"});
+
+ final ResourceMapper mapper = mock(ResourceMapper.class);
+
when(mapper.getAllMappings("/path1")).thenReturn(Collections.singleton("/path1"));
+
when(mapper.getAllMappings("/path2")).thenReturn(Collections.singleton("/path2"));
+
+ final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
+ config, callable -> callable.run());
+
+ assertPaths(manager, new String[] {"/path1", "/path2"},
+ new ServiceReference<?>[] {null, null},
+ new boolean[] {true, false});
+
+ }
}
diff --git
a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
index 6dfc3a3..4e8ca72 100644
--- a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
+++ b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
@@ -34,6 +34,7 @@ import org.apache.sling.commons.metrics.MetricsService;
import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import junitx.util.PrivateAccessor;
@@ -64,9 +65,19 @@ public class SlingAuthenticatorTest {
return createSlingAuthenticator(createDefaultConfig(),
typeAndPathPairs);
}
+ private static final long BUNDLE_ID = 732;
+
+ private BundleContext createBundleContext() {
+ final BundleContext context = Mockito.mock(BundleContext.class);
+ final Bundle bundle = Mockito.mock(Bundle.class);
+ Mockito.when(bundle.getBundleId()).thenReturn(BUNDLE_ID);
+ Mockito.when(context.getBundle()).thenReturn(bundle);
+ return context;
+ }
+
private SlingAuthenticator createSlingAuthenticator(final
SlingAuthenticator.Config config,
final String... typeAndPathPairs) {
- final AuthenticationRequirementsManager requirements = new
AuthenticationRequirementsManager(Mockito.mock(BundleContext.class), null,
config, callable -> callable.run());
+ final AuthenticationRequirementsManager requirements = new
AuthenticationRequirementsManager(createBundleContext(), null, config, callable
-> callable.run());
final AuthenticationHandlersManager handlers = new
AuthenticationHandlersManager(config);
if ( typeAndPathPairs != null ) {
int i=0;
