Hi,
Am 20.09.2010 17:16, schrieb Justin Edelson:
> Two comments:
>
> 1) As mentioned, I think we should use the Accepts header instead of
> specifically checking for the OPTIONS method and return a 401 response
> unless the Accepts header is included in the request and *explicitly*
> contains text/html. Since I don't have the time right now to research
> this, could you just pull out this block:
As I said, this is about WebDAV clients which (I would assume, could/did
not find anything concrete in this respect, but this is what my
experience tells) start communication with an OPTIONS request. Probably
to find out about DAV support (OPTIONS response must have a DAV response
header indicating the level of DAV support).
Such an OPTIONS request will generally not have an Accepts header. But
we could use the Accepts header check as negative check: Assume WebDAV
OPTIONS request if Accepts header is not present.
>
> + if (HttpConstants.METHOD_OPTIONS.equals(request.getMethod())) {
> +
> + // Presumably this is WebDAV. If HTTP Basic is fully enabled or
> + // enabled for preemptive credential support, we just request
> + // HTTP Basic credentials. Otherwise (HTTP Basic is fully
> + // switched off, 403 is sent back)
> + if (httpBasicHandler != null) {
> + httpBasicHandler.sendUnauthorized(response);
> + return;
> + }
>
> into a "shouldReturnUnauthorizedResponse" method? That way, where this
> logic gets change will be more obvious.
Makes sense. For consistency I would then also craete a method "isAjax"
(or such) for the other test.
>
> 2) As Ian suggested, we should include the form path into the response
> somehow. We could use the Location header, but perhaps we should use a
> custom header like X-Login-Form.
>
> The goal of #2 is to allow a JavaScript client to do either (a) show a
> custom login dialog or (b) redirect the user to the login form
> (presumably keeping some kind of state).
Makes sense - I just did not add this yet to the proposed patch. I would
propose to use a custom header instead of reusing the Location header,
which has specific meaning according to RFC 2616.
>
> Might also be a good idea to get some feedback on this from a Flex
> person. IIRC, there were weird issues with 401/403 responses with Flex.
We might want to treat Flex similar to Ajax, probably.
Regards
Felix
>
> Justin
>
> On 9/20/10 10:19 AM, Felix Meschberger wrote:
>> Hi all,
>>
>> I have uploaded a proposed patch including support for both issues to
>> http://codereview.appspot.com/2192046/.
>>
>> Please comment. Thanks.
>>
>> Regards
>> Felix
>>
>>
>>
>> Am 17.09.2010 16:59, schrieb Felix Meschberger:
>>> Hi all,
>>>
>>> I am trying to tackle issues SLING-1400 [1] and SLING-1745 [2].
>>>
>>> The first issue is about WebDAV clients connecting to Sling on root with
>>> an OPTIONS request and not being happy with a redirect response, obviously.
>>>
>>> The second issue is about client side JavaScript application framework
>>> which may send XHR requests to Sling, mainly POSTs destined for the POST
>>> Servlet but probably also other stuff. Such framework are also generally
>>> not very happy getting redirect responses back.
>>>
>>> Solutions for both problems would probably have to be implemented in the
>>> SlingAuthenticator.doLogin method, which is called after an unsuccessful
>>> login or after a first request noticing that authentication is required.
>>>
>>> So here are the options I came up with:
>>>
>>> * Send back a 401 response, at least for the OPTIONS request
>>> to trigger a regular HTTP Basic Authentication
>>> * Send back a 403 response, to indicate that access is currently
>>> forbidden (we discussed this option earlier [3]).
>>>
>>> My questions:
>>>
>>> - Would it be ok to special case the OPTIONS request ?
>>> - Shall we generally only send back a generic credentials request
>>> (may be a redirect or a form directly or whatever) if the
>>> original request was GET and send back either 401 or 403 for
>>> all non-GET requests, including HEAD ?
>>> - Is it a good idea to send back 401 generally ?
>>> - Should we only send back 401 if HTTP Basic authentication is
>>> at enabled fully or enabled preemptively and send back 403 if
>>> HTTP Basic authentication is switched off completely ?
>>> - Am I completely off track ?
>>>
>>> WDYT ?
>>>
>>> Regards
>>> Felix
>>>
>>>
>>>
>>> [1] https://issues.apache.org/jira/browse/SLING-1400
>>> [2] https://issues.apache.org/jira/browse/SLING-1745
>>> [3] http://markmail.org/message/jwsvk6swnxvvfsyz
>
>