[jira] [Commented] (SLING-13353) SlingAuthenticator: unbounded handleLoginFailure <-> getAnonymousResolver recursion when anonymous login repeatedly fails

["Rishabh Daim (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-13353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18124348#comment-18124348 ] 

Rishabh Daim commented on SLING-13353:
--------------------------------------

h3. AI-reviewed assessment

*Disclaimer:* This comment was prepared with AI assistance and should be confirmed by the project maintainers.

I reviewed the [MAIA review|https://github.com/apache/sling-org-apache-sling-auth-core/pull/29#pullrequestreview-5439354322] against the merged implementation.

*Medium finding — marker lifetime:* I could not validate this as a concrete issue in the supported authentication flow. {{handleSecurity}} returns early when a resolver is already attached to the request, and successful anonymous resolution stores that resolver on the request. I found no in-repository path where a normal include or forward reaches {{handleLoginFailure}} again after successful resolution. The concern remains a theoretical edge case if a caller re-enters authentication after removing the resolver attribute. I do not recommend a production change for the merged PR without an integration test reproducing that lifecycle.

*Low finding — test structure:* This is a valid optional test-quality suggestion. The regression test uses private reflection, consistent with existing tests, and verifies the key failure mode: one anonymous login attempt with no recursion. It could be strengthened by asserting the terminal error or login-response behavior, but this is not a production defect.

*Verification:* The targeted tests passed 40/40 on Java 17; the full suite had previously passed 304 tests. Repository history also showed no earlier fix for this exact recursion; the older {{ReadOnlyAuthenticationInfo.stackGuard}} addresses a separate issue.

No corrective action is required based on this review. A follow-up integration test could be added if the project wants to validate re-entry after resolver cleanup.

> SlingAuthenticator: unbounded handleLoginFailure <-> getAnonymousResolver recursion when anonymous login repeatedly fails
> -------------------------------------------------------------------------------------------------------------------------
>
>                 Key: SLING-13353
>                 URL: https://issues.apache.org/jira/browse/SLING-13353
>             Project: Sling
>          Issue Type: Bug
>    Affects Versions: Auth Core 2.0.2
>            Reporter: Rishabh Daim
>            Priority: Major
>
> h4. Problem
> {{SlingAuthenticator}} recurses without bound between {{handleLoginFailure}} and {{getAnonymousResolver}} when acquiring the anonymous {{ResourceResolver}} keeps failing with {{LoginException}} (e.g. the backing repository is unavailable). The stack grows until {{StackOverflowError}}, and since each failed request logs the full repeating trace, a sustained failure floods the error log (observed in the field growing until the disk filled).
> h4. Cause
> In {{org.apache.sling.auth.core.impl.SlingAuthenticator}}, the two methods call each other with no termination guard:
> * {{getAnonymousResolver}}: on {{LoginException}} from {{getResourceResolver}}, calls {{handleLoginFailure}};
> * {{handleLoginFailure}}: for a {{LoginException}} with anonymous access still allowed, calls {{getAnonymousResolver}}.
> So {{getAnonymousResolver -> handleLoginFailure -> getAnonymousResolver -> ...}} repeats as long as the anonymous login keeps failing. The anonymous fallback is never marked as already-attempted.
> h4. Reproduce
> Anonymous access allowed; make {{getResourceResolver}} throw {{LoginException}}; issue a plain request to an anonymous path. Expected: one anonymous attempt, then terminate. Actual: recursion to {{StackOverflowError}}.
> h4. Fix
> Per-request re-entrancy guard: {{getAnonymousResolver}} marks (request attribute) that anonymous resolution was attempted; {{handleLoginFailure}} only falls back when the mark is absent - at most one attempt per request, terminating normally on repeated failure. All currently-successful paths (including a single legitimate anonymous fallback) are unchanged. Regression test drives repeated {{LoginException}} and asserts no {{StackOverflowError}} and exactly one {{getResourceResolver}} call; it fails without the guard.
> Affects current trunk (auth-core bundle 66) and earlier releases with the same structure.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)