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

Andrei Dulvac commented on SLING-13025:
---------------------------------------

[~enorman] thanks for catching this. The NPE is fixed in master under 
SLING-13389 (commit 00dd9d2): getMappedLoginResourcePath returns the unmapped 
path when no resolver is on the request, and getAnonymousResolver only 
redirects on an explicit sling.auth.redirect. The resource fallback is meant 
for the redirect after a successful login; on the anonymous path the request is 
usually the login form itself, so redirecting it to the protected resource 
would loop. The 2.1.0 vote is cancelled, next candidate is 2.1.2.

[~kwin] while testing the fix in the Starter I found a second side effect of 
this change. Every successful form login with a resource parameter now logs

{noformat}
*WARN* SlingAuthenticator handleSecurity: AuthenticationHandler did not block 
request; access denied
*ERROR* org.apache.felix.http Exception while processing request to 
/j_security_check
java.lang.IllegalStateException: Response is already committed
    at org.apache.sling.auth.core.AuthUtil.checkAndReset(AuthUtil.java:1157)
    at org.apache.sling.auth.core.AuthUtil.sendInvalid(AuthUtil.java:678)
    at 
org.apache.sling.auth.core.impl.SlingAuthenticator.handleSecurity(SlingAuthenticator.java:499)
{noformat}

The client still gets the right 302, only the log is affected. Cause: 
FormAuthenticationHandler.authenticationSucceeded calls 
DefaultJakartaAuthenticationFeedbackHandler.handleRedirect and returns false 
when that returns true ("terminate request, all done in the default handler"), 
so SlingAuthenticator keeps processing and then tries to send a 403 on a 
committed response. With 2.0.2 that path was only hit for sling.auth.redirect 
(same WARN and ERROR there, I checked against 2.0.2 in the same Starter 
instance). Now that handleRedirect also fires for resource, it is hit on every 
form login.

Fixed as well under SLING-13389: SlingAuthenticator.getResolver treats a 
committed response as the end of the request even when the feedback handler 
returned false, so this is covered for any feedback handler. The inverted 
return in sling.auth.form deserves its own fix too; I will open an issue for it.


> Default(Jakarta)AuthenticationFeedbackHandler should evaluate "resource" 
> parameter as fallback for "sling.auth.redirect"
> ------------------------------------------------------------------------------------------------------------------------
>
>                 Key: SLING-13025
>                 URL: https://issues.apache.org/jira/browse/SLING-13025
>             Project: Sling
>          Issue Type: Improvement
>          Components: Authentication Core
>    Affects Versions: Auth Core 2.0.2
>            Reporter: Konrad Windszus
>            Assignee: Konrad Windszus
>            Priority: Major
>             Fix For: Auth Core 2.1.2
>
>
> As outlined in 
> https://sling.apache.org/documentation/the-sling-engine/authentication/authentication-authenticationhandler/form-based-authenticationhandler.html#phase-1-form-submission
> {quote}
> The resource and sling.auth.redirect parameters provide similar functionality 
> but with differing historical backgrounds. The resource parameter is based on 
> the resource request attribute which is set by the login servlet to indicate 
> the original target resource the client desired when it was forced to 
> authenticate. The sling.auth.redirect parameter can be used by clients 
> (applications like cURL or plain HTML forms) to request being redirected 
> after successful login. If both parameters are set, the sling.auth.redirect 
> parameter takes precedence.
> {quote}
> However the 
> [DefaultJakartaAuthenticationFeedbackHandler|https://github.com/apache/sling-org-apache-sling-auth-core/blob/b8409ee840277cfaeb1f58c8648259b811f7789e/src/main/java/org/apache/sling/auth/core/spi/DefaultJakartaAuthenticationFeedbackHandler.java#L32C14-L32C57]
>  and 
> [DefaultAuthenticationFeedbackHandler|https://github.com/apache/sling-org-apache-sling-auth-core/blob/b8409ee840277cfaeb1f58c8648259b811f7789e/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java#L33]
>  only evaluate {{sling.auth.redirect}}.
> In order to reduce the amount of parameters necessary it would be good to 
> issue a redirect considering {{resource}} in case {{sling.auth.redirect}} is 
> not set (as in most cases it is the desired behaviour to redirect to the 
> resource which originally triggered the login).



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

Reply via email to