rishabhdaim commented on PR #29: URL: https://github.com/apache/sling-org-apache-sling-auth-core/pull/29#issuecomment-6033932717
## 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
