Hi, Am 22.09.2010 19:14, schrieb Justin Edelson: > BTW, for WebDAV clients, I tested on Ubuntu 10.04/Gnome VFS and OS X > 10.6.4/Finder.
Thanks, will add the OS X finder to the list of tested clients in the javadoc. Regards Felix > > On Wed, Sep 22, 2010 at 12:39 PM, Justin Edelson > <[email protected]> wrote: >> 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 >>>>>> >>>>>> >>>> >>>> >> >> >
