You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2010/09/17 16:59:30 UTC

Authentication Issue

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

Re: Authentication Issue

Posted by Ian Boston <ie...@tfd.co.uk>.
On 21 Sep 2010, at 00:19, 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.

The patch addresses all my earlier concerns with Ajax calls, thank you.
I can see that Justin has some additional questions.
Ian



Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
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
>>>>>
>>>>>
>>>
>>>
> 
> 

Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
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
> <ju...@gmail.com> 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
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
> 

Re: Authentication Issue

Posted by Justin Edelson <ju...@gmail.com>.
BTW, for WebDAV clients, I tested on Ubuntu 10.04/Gnome VFS and OS X
10.6.4/Finder.

On Wed, Sep 22, 2010 at 12:39 PM, Justin Edelson
<ju...@gmail.com> 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
>>>>>
>>>>>
>>>
>>>
>
>

Re: Authentication Issue

Posted by Justin Edelson <ju...@gmail.com>.
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
>>>>
>>>>
>>
>>


Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
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
>>>
>>>
> 
> 

Re: Authentication Issue

Posted by Justin Edelson <ju...@gmail.com>.
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
>>
>>


Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
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
> 
> 

Re: Authentication Issue

Posted by Justin Edelson <ju...@gmail.com>.
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


Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
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

Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

Am 17.09.2010 18:09, schrieb Justin Edelson:
> At least in the WebDAV case, is there any way to use the Accepts header
> to help with making the decision as to how to respond? i.e. if Accepts
> contains text/html, return the login page. If it doesn't, return a 401.
> Perhaps this requires more testing of WebDAV clients than we can
> effectively do, but it does seem to make semantic sense - if a user
> agent says it can support (accept) HTML, we should give it HTML.

Sounds like a good idea.

But: The first request of WebDAV clients generally is an OPTIONS request
which does not have much headers except Host and User-Agent.

> 
> Doesn't really help with XHR, but I thought the 403 response was the
> right way to deal with that.

Yes.

Regards
Felix

> 
> Justin
> 
> On 9/17/10 10:59 AM, Felix Meschberger wrote:
>> 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
> 
> 

Re: Authentication Issue

Posted by Justin Edelson <ju...@gmail.com>.
At least in the WebDAV case, is there any way to use the Accepts header
to help with making the decision as to how to respond? i.e. if Accepts
contains text/html, return the login page. If it doesn't, return a 401.
Perhaps this requires more testing of WebDAV clients than we can
effectively do, but it does seem to make semantic sense - if a user
agent says it can support (accept) HTML, we should give it HTML.

Doesn't really help with XHR, but I thought the 403 response was the
right way to deal with that.

Justin

On 9/17/10 10:59 AM, Felix Meschberger wrote:
> 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


Re: Authentication Issue

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

Am 17.09.2010 23:48, schrieb Ian Boston:
> 
> On 18 Sep 2010, at 00:59, Felix Meschberger wrote:
> 
>> 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 ?
> 
> IMHO Yes, as AFAIK its only normally seen with webdav.
> 
>>  - 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 ?
> 
> The credentials request should include something that doesn't need to be parsed to indicate it is a credentials request. (header or status code). 

Hmm, adding an X-Reason header duplicating the value of the (optional)
j_reason parameter from the redirect response sounds like an option here.

> 
> Redirect or form is Ok provided the request is not Ajax. If it is, then who knows what the client will do, in which case status code is really the only correct way of responding. Can you do a 401/403 and the html, redirect in as a meta-equiv if required.

Not sure, whether I understand.

As for the 401 response we can probably not add anything be
cause the browser immediately displays the regular browser login box
without much ado.

For 403 it might be an option, indeed to provide information as to what
URL to request to allow the client to login possibly having the client
open another window for this action....

> 
> 
>>  - 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 ?
> 
> This prompted me to go and look at the spec[1].
> 401 must be have a WWW-Authenticate which will cause the login box to appear (iirc) even with Ajax requests.
> And there is no way to know if the client wants to see a login box. Could be very confusing for anyone with SSO or Form based auth.

That's exactly why I am reluctant to just send back 401...

I will post a patch to codereview.appspot for further discussion.

Regards
Felix

> 
> Sending a bare 401 might be the right thing to do where the user is anon, but that would be illegal, perhaps the client can work out that they are not logged in, which makes 403 Ok.
> 
> Ian
> 
> 
> 1 http://www.freesoft.org/CIE/RFC/2068/209.htm
> 
>>  - 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
> 
> 

Re: Authentication Issue

Posted by Ian Boston <ie...@tfd.co.uk>.
On 18 Sep 2010, at 00:59, Felix Meschberger wrote:

> 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 ?

IMHO Yes, as AFAIK its only normally seen with webdav.

>  - 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 ?

The credentials request should include something that doesn't need to be parsed to indicate it is a credentials request. (header or status code). 

Redirect or form is Ok provided the request is not Ajax. If it is, then who knows what the client will do, in which case status code is really the only correct way of responding. Can you do a 401/403 and the html, redirect in as a meta-equiv if required.


>  - 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 ?

This prompted me to go and look at the spec[1].
401 must be have a WWW-Authenticate which will cause the login box to appear (iirc) even with Ajax requests.
And there is no way to know if the client wants to see a login box. Could be very confusing for anyone with SSO or Form based auth.

Sending a bare 401 might be the right thing to do where the user is anon, but that would be illegal, perhaps the client can work out that they are not logged in, which makes 403 Ok.

Ian


1 http://www.freesoft.org/CIE/RFC/2068/209.htm

>  - 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