Hi,
Ok, sounds reasonable, I will do it like this.
Thanks alot.
Regards
Felix
Am 22.09.2010 18:39, schrieb Justin Edelson:
> I commented on the code inline, but to clarify here, what I was trying
> to suggest below is to ignore the request method and *only* using the
> Accepts header.
>
> To me, the logic I think we should be expressing is:
>
> if (isBrowserRequest(request)) {
> if (isAjaxRequest(request)) {
> return 403;
> } else {
> hand off to AuthenticationHandler;
> }
> } else {
> return 401;
> }
>
> The login page is a web browser-specific notion. Any other HTTP client
> should really get a 401 response. This would include curl, wget, mobile
> apps, etc...
>
> I posted an alternative patch here: http://codereview.appspot.com/2252043
>
> This also fixes SLING-1428, which isn't strictly related; the reason for
> this inclusion is that the test is the basically the same. This can, of
> course, be peeled off into a separate commit.
>
> Justin
>
> On 9/21/10 4:11 AM, Felix Meschberger wrote:
>> Hi,
>>
>> Ok, I have uploaded another patch with two methods to check for WebDAV
>> initial request and Ajax request.
>>
>> I for now left out any more intense Accepts header testing since none of
>> the WebDAV clients I tested with (see JavaDoc) sent an Accepts header at
>> all.
>>
>> I also ommited adding a header with an URL to use for login. For one the
>> main URL to be used is part of the LoginServlet configuration (outside
>> of the control of the SlingAuthenticator class) and second because just
>> sending a regular GET request to the same URL should actually also
>> trigger a login redirect. In essence: there is no use in such a header.
>>
>> As for Flex: This is really a sad situation currently. For now, I do not
>> know how to deal with this and would like to leave it out. We can still
>> fix it once someone comes along with more insight.
>>
>> Regards
>> Felix
>>
>> Am 20.09.2010 21:44, schrieb Justin Edelson:
>>> 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
>>>>>
>>>>>
>>>
>>>
>
>
