On 6/22/10 9:28 AM, Felix Meschberger wrote:
> Hi,
>
> On 22.06.2010 15:09, Justin Edelson wrote:
>> On 6/22/10 3:22 AM, Felix Meschberger wrote:
>>> Hi,
>>>
>>> The advantage of a redirect is, that the redirect target can fully
>>> leverage all Sling functionality.
>>>
>>> At the time the AuthenticationHandler.requestCredentials method is
>>> called the SlingHttpServletRequest and *Response objects have not been
>>> set up yet and no resource resolver is available.
>>>
>>> Thus, a redirect is the only really feasible and useful option.
>> Hi,
>> I wasn't clear enough in the issue summary. What I'm proposing is an
>> option with the default being to redirect and to ONLY support Servlet
>> resources, i.e.
>>
>> Resource loginFormResource = resourceResolver.getResource(loginForm);
>
> So, the authentication handler would have a reference to the
> ResourceResolverFactory and create its own ResourceResolver for this ?
Correct.
>
>> Servlet loginFormServlet = loginFormResource.adaptTo(Servlet.class);
>
> This might actually also return scripts masqueraded as Servlets ;-)
Sure, but in order for this to be the case, someone will need to have
explicitly configured the form path to point to a script.
>
> The problem though remains, that the servlet (or script) is actually
> called with HttpServletRequest/Response and not
> SlingHttpServletRequest/Response. This may not be actually a problem if
> the servlet/script is prepared for this....
I don't see this as a problem. It would be if include was the default,
but I think we should keep redirect as the default.
The other thing I noticed is that the servlet needs to handle POST as
well as GET to handle a failed authentication.
>
> So you would keep the login form path and do something like this:
>
> resource = resolve(formPath);
> servlet = (resource != null)
> ? resource.adaptTo(Servlet.class);
> : null;
> if (servlet != null) {
> // set helper attributes (j_reason, etc.)
> servlet.service(...);
> } else {
> response.sendRedirect(formPath);
> }
This isn't exactly how the code looks, but it is pretty close
conceptually. There is first a check to see if the include option is
enabled. If it is, the resource resolution/adaptation test happens and
then the service method is called.
If the include option is disable or the resolution/adaptation fails, the
redirect URL is built and then the redirect happens.
Once I pull in your changes to formauth, I'll recreate the patch and
post it somewhere for review.
One other thing I considered is if there was a way to simply have a
different service interface which allowed for the content of the form
page to be overridden; as things stand, it is a bit of an abuse of the
Servlet interface, but Sling is all about abusing the Servlet interface :)
>
> On a related issue: we might want to provide an
> AbstractAuthenticationHandler which provides common functionality
> currently duplicated in the Form- and OpenIDAuthenticationHandlers.
This seems like a good idea to me.
Justin
>
> WDYT ?
>
> Regards
> Felix
>
>
>> if (loginFormServlet != null) {
>> try {
>> loginFormServlet.service(request, response);
>> return true;
>> } catch (ServletException e) {
>> log.error("Failed to include the form: " + loginForm, e);
>> }
>> }
>>
>> Redirects have two problems:
>> 1) They require an additional network round trip
>> 2) They lose state
>>
>> #2 is the problem I'm facing in the near term - as things currently
>> stand, the formauth bundle will not pass through the query and search
>> portions of the request path. As a result, the original URL (say, a
>> bookmark) cannot be fully reconstructed. This is a solvable problem for
>> the query string, but the only way to get the search portion of the URL
>> is on the client side, which isn't possible without doing an include of
>> the login servlet form.
>
>>
>> Justin
>>
>>>
>>> Regards
>>> Felix
>>>
>>> On 22.06.2010 04:05, Justin Edelson (JIRA) wrote:
>>>> have the form authentication handler include the login form as a resource
>>>> rather than doing a redirect
>>>> ------------------------------------------------------------------------------------------------------
>>>>
>>>> Key: SLING-1564
>>>> URL: https://issues.apache.org/jira/browse/SLING-1564
>>>> Project: Sling
>>>> Issue Type: Improvement
>>>> Components: Extensions
>>>> Reporter: Justin Edelson
>>>>
>>>>
>>>>
>>>>
>>
>>
