On 9/20/10 1:56 PM, Felix Meschberger wrote:
> 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.
Right, I'm suggesting that if a request doesn't have an Accepts header,
give it a 401.
Accepts value Response code
------------- -------------
(no header) 401
*/* 401
text/xml 401
text/html 302 or 403 (depends on X-Requested-With)
text/xml,text/html 302 or 403 (depends on X-Requested-With)
I would assume that a WebDAV client would either not use the Accepts
header or pass */*. But, as I said, I haven't had the time to test this.
>
>>
>> + 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.
Agreed.
>>
>> 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.
Well, I think this use case falls under the spec, but I don't feel
strongly about this one way or the other.
>
>>
>> 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.
I don't think this will work (at least it wouldn't work a year ago,
maybe things have changed). When running inside the browser, any non-200
response status appears to the Flash runtime as a 500. I don't know if
there's an equivalent to X-Requested-With: XmlHttpRequest for Flash
clients. This is apparently a browser limitation (see
http://bugs.adobe.com/jira/browse/SDK-11841).
Justin
>
> 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
>>
>>
