Hi,
On 22.06.2010 15:42, Justin Edelson wrote:
> 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.
yes, just set the path to /some/path.jsp or so.
>
>>
>> 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.
I see. Ok, sounds good.
>
> 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 :)
You mean the static HTML that the Form Servlet returns ?
Yes I have been thinking of this, too.... But I couldn't come up with
good API to do it -- maybe some Fragment based resource "injection" or
some ResourceResolver provided resource ?
>>
>> 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.
Ok, I'l try to come up with something; probably to be placed into the
Commons Auth bundle (in the spi package)
Regards
Felix
>
> 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
>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>
>
