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 40643db SLING-11867 - Empty mapping from ResourceMapper breaks
authentication… (#14)
40643db is described below
commit 40643dbc4b28b446f488997f5cab150b6e449249
Author: Sagar Miglani <85228812+sagarmigl...@users.noreply.github.com>
AuthorDate: Wed May 17 11:42:16 2023 +0530
SLING-11867 - Empty mapping from ResourceMapper breaks authentication… (#14)
* SLING-11867 - Empty mapping from ResourceMapper breaks authentication
requirement updates
* SLING-11867 - Empty mapping from ResourceMapper breaks authentication
requirement updates
---------
Co-authored-by: Sagar Miglani <s...@adobe.com>
---
.../impl/AuthenticationRequirementsManager.java | 14 +++++++--
.../AuthenticationRequirementsManagerTest.java | 36 ++++++++++++++++++++++
2 files changed, 48 insertions(+), 2 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 4e88ec3..2d84fab 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
@@ -158,6 +158,8 @@ public class AuthenticationRequirementsManager
if (authReq != null && authReq.length() > 0) {
this.addHolder(AuthenticationRequirementHolder.fromConfig(
authReq, null));
+ } else {
+ logger.warn("Ignoring null/empty config for auth
requirements");
}
}
}
@@ -371,6 +373,10 @@ public class AuthenticationRequirementsManager
if ( !paths.isEmpty() ) {
final List<AuthenticationRequirementHolder> authReqList = new
ArrayList<>();
for(final String authReq : paths) {
+ if (authReq == null || authReq.isEmpty()) {
+ logger.warn("Ignoring null/empty path while adding
auth requirements for service {}", id);
+ continue;
+ }
authReqList.add(AuthenticationRequirementHolder.fromConfig(authReq, ref));
}
@@ -402,7 +408,9 @@ public class AuthenticationRequirementsManager
final List<AuthenticationRequirementHolder> authReqs =
props.get(id);
// compare sets
for(final String oldPath : oldPaths) {
- if ( !paths.contains(oldPath) ) {
+ if (oldPath == null || oldPath.isEmpty()) {
+ continue;
+ } else if ( !paths.contains(oldPath) ) {
// remove
final AuthenticationRequirementHolder holder =
AuthenticationRequirementHolder.fromConfig(oldPath, ref);
authReqs.remove(holder);
@@ -410,7 +418,9 @@ public class AuthenticationRequirementsManager
}
}
for(final String path : paths) {
- if ( !oldPaths.contains(path) ) {
+ if (path == null || path.isEmpty()) {
+ logger.warn("Ignoring null/empty path while updating
the auth requirements for service {}", id);
+ } else if ( !oldPaths.contains(path) ) {
// add
final AuthenticationRequirementHolder holder =
AuthenticationRequirementHolder.fromConfig(path, ref);
authReqs.add(holder);
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 6082252..86ca1af 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
@@ -310,6 +310,42 @@ public class AuthenticationRequirementsManagerTest {
assertEquals(3, manager.getHolders().size());
}
+ // see SLING-11867
+ @Test public void testRegistrationWithEmptyMapping() throws LoginException
{
+ final BundleContext context = createBundleContext();
+ final ResourceMapper mapper = mock(ResourceMapper.class);
+
+ // Resourcemapper returns empty mapping
+
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1", ""));
+
+ final AuthenticationRequirementsManager manager = new
AuthenticationRequirementsManager(context, createFactoryForMapper(mapper),
+ SlingAuthenticatorTest.createDefaultConfig(), callable ->
callable.run());
+
+ // register
+ final ServiceReference<?> ref = createServiceReference(new String[]
{"+/path1"});
+ manager.serviceChanged(new ServiceEvent(ServiceEvent.REGISTERED, ref));
+
+ // empty path "" returned by resourcemapper should have been ignored
+ assertPaths(manager, new String[] {"/path1"},
+ new ServiceReference<?>[] {ref},
+ new boolean[] {true});
+
+ // update mapper
+
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1",
"/path5"));
+ manager.handleEvent(null);
+ assertPaths(manager, new String[] {"/path1", "/path5"},
+ new ServiceReference<?>[] {ref, ref},
+ new boolean[] {true, true});
+
+ // update mapper - now again with empty mapping to test the case of
modified
+ // empty "" should be ignored
+
when(mapper.getAllMappings("/path1")).thenReturn(Arrays.asList("/path1", ""));
+ manager.handleEvent(null);
+ assertPaths(manager, new String[] {"/path1"},
+ new ServiceReference<?>[] {ref},
+ new boolean[] {true});
+ }
+
@Test public void testAllowDeny() throws LoginException {
final BundleContext context = createBundleContext();
