This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits 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 7f52a63  SLING-13389 : terminate the request when a feedback handler 
committed the response
7f52a63 is described below

commit 7f52a637d8a26b3ed3696b1eada27e9df4a7ebb9
Author: dulvac <[email protected]>
AuthorDate: Tue Oct 13 16:34:09 2026 +0200

    SLING-13389 : terminate the request when a feedback handler committed the 
response
    
    sling.auth.form's authenticationSucceeded sends the redirect and then
    returns false. Since SLING-13025 that happens on every form login with a
    "resource" parameter (before only with sling.auth.redirect), and
    SlingAuthenticator kept processing: handleSecurity logged
    "AuthenticationHandler did not block request" and sendInvalid threw
    IllegalStateException on the committed response, once per login.
    
    getResolver now treats a committed response as the end of the request
    even when the feedback handler returned false.
---
 .../sling/auth/core/impl/SlingAuthenticator.java   |  9 +++++
 .../auth/core/impl/SlingAuthenticatorTest.java     | 47 +++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git 
a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java 
b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
index 340519f..2e1f5c9 100644
--- a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
+++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
@@ -846,6 +846,15 @@ public class SlingAuthenticator implements Authenticator, 
AuthenticationSupport,
             // custom feedback handler with option to redirect
             if (feedbackHandler != null) {
                 processRequest = 
!feedbackHandler.authenticationSucceeded(request, response, authInfo);
+                if (processRequest && response.isCommitted()) {
+                    // the handler sent a response (e.g. a redirect) but asked 
us to
+                    // continue; nothing can be processed on a committed 
response,
+                    // so terminate the request instead of running into 
sendInvalid
+                    log.debug(
+                            "getResolver: feedback handler {} committed the 
response but returned false, terminating request",
+                            feedbackHandler.getClass().getName());
+                    processRequest = false;
+                }
             }
 
             if (processRequest) {
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 e0f4af0..430f489 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
@@ -47,6 +47,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.eq;
@@ -525,10 +526,15 @@ public class SlingAuthenticatorTest {
     }
 
     private AbstractAuthenticationHandlerHolder infoHolder(final String path, 
final AuthenticationInfo info) {
+        return infoHolder(path, info, null);
+    }
+
+    private AbstractAuthenticationHandlerHolder infoHolder(
+            final String path, final AuthenticationInfo info, final 
JakartaAuthenticationFeedbackHandler feedback) {
         return new AbstractAuthenticationHandlerHolder(path, null) {
             @Override
             protected JakartaAuthenticationFeedbackHandler 
getFeedbackHandler() {
-                return null;
+                return feedback;
             }
 
             @Override
@@ -763,6 +769,45 @@ public class SlingAuthenticatorTest {
         verify(resolver).close();
     }
 
+    @Test
+    public void 
test_handleSecurity_feedback_handler_committed_response_but_returned_false() 
throws Exception {
+        // sling.auth.form's authenticationSucceeded sends the redirect 
(committing the response) and
+        // then returns false; the authenticator must treat the request as 
done instead of continuing
+        // into "AuthenticationHandler did not block request" and sendInvalid 
on a committed response
+        final ResourceResolverFactory rrf = 
mock(ResourceResolverFactory.class);
+        final ResourceResolver resolver = mock(ResourceResolver.class);
+        
when(rrf.getResourceResolver(any(java.util.Map.class))).thenReturn(resolver);
+
+        final HttpServletResponse response = mock(HttpServletResponse.class);
+        final JakartaAuthenticationFeedbackHandler feedback = new 
JakartaAuthenticationFeedbackHandler() {
+            @Override
+            public void authenticationFailed(
+                    HttpServletRequest request, HttpServletResponse response, 
AuthenticationInfo authInfo) {}
+
+            @Override
+            public boolean authenticationSucceeded(
+                    HttpServletRequest request, HttpServletResponse response, 
AuthenticationInfo authInfo) {
+                try {
+                    response.sendRedirect("/content/target");
+                } catch (IOException e) {
+                    throw new IllegalStateException(e);
+                }
+                when(response.isCommitted()).thenReturn(true);
+                return false;
+            }
+        };
+        final AuthenticationInfo info = new AuthenticationInfo("form", "user", 
"pwd".toCharArray());
+        final SlingAuthenticator auth = createAuthenticator(rrf, 
infoHolder("/", info, feedback));
+
+        final HttpServletRequest request = requestFor("/j_security_check");
+
+        Assert.assertFalse(auth.handleSecurity(request, response));
+        verify(response).sendRedirect("/content/target");
+        verify(response, never()).sendError(anyInt(), anyString());
+        verify(response, never()).setStatus(anyInt());
+        verify(resolver).close();
+    }
+
     @Test
     public void test_handleSecurity_doing_auth() {
         final SlingAuthenticator auth = createAuthenticator(infoHolder("/", 
AuthenticationInfo.DOING_AUTH));

Reply via email to