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();
 

Reply via email to