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:
+ 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.
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).
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.
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
