Hi devs,

There seems to be a problem with the way the login cookies are handled 
in the latest code.

The problem was introduced in order to fix XWIKI-1655, XE-91 and similar 
issues. The failing scenario was:
- wiki A sending cookies for .company.com
- wiki B sending cookies for my.company.com
- Login in wiki A with a user that doesn't exist in wiki B
- Access wiki B and try to login

The user will not be able to login on wiki B, unless he logouts from 
wiki A, changes the browser, or manually removes the cookies. This is 
caused by the fact that both cookies will be sent by the client, and 
XWiki uses only the first value found.

My initial fix was to remove cookies from all possible domain names 
matching the current URL, so when accessing www.my.wiki.net, on login 
invalidation the server would send cookie removal requests from the 
following domains: <empty>, "www.my.wiki.net", "my.wiki.net", "wiki.net".

However, this causes problems because container implementations are 
wrong. The cookie spec says that a server can request at least as much 
as 20 cookies for each domain, and each cookie should support at least 
4096 bytes. While we do stay in these limits, containers allow only 4096 
bytes for ALL cookies sent in one request, instead of 4096 for each 
cookie (tomcat allows 8192). And when using a domain name with many dots 
inside, there's an exception that prevents the page from being displayed.

Because we can't change the containers, we have to find another solution.

I have improved the current code a bit, by:
- removing duplicate calls to forgetLogin (yep, containers are stupid 
enough not to check if the same cookie was set more than once)
- if there were no cookies, don't request to remove them (yep, XWiki did 
that)
- using not the first value found for a cookie, but the last one, as 
most popular browsers send cookie in the order they were set, so in the 
example above the cookies set by wiki B will be picked instead of those 
set by wiki A (this flips which browsers fail the above scenario, for 
example now it works on FF and Safari, but fails on Links and Opera)

I could still improve:
- override ServletResponse to detect duplicate cookies
- remove only some of the cookies (username and password), as the other 
cookies alone cannot be used

Still, this is not enough to ensure that the 4096 limit will never be 
reached. If there's a really long domain name, say with 10 dots, then 
there will be too many cookies.

Thus, I propose to revert this exhaustive cookie removal. While this 
will imply that the scenario above will continue to fail on some 
browsers, I think that this scenario shows an administration error, and 
not an XWiki error. Either wiki B should be a virtual wiki, or wiki A 
should not set cookies for all the subdomains.

Ludovic suggested another fix, which is to prefix cookie names with the 
domain name. I don't like this option very much for the default 
behavior. I agree that it should be an optional feature.

So, my proposal is to:
- revert the exhaustive cookie removal, and stick to the old behavior 
(remove cookies for the <empty> domain and the configured cookieDomain, 
if any)
- add a cookiePrefix parameter

WDYT?
-- 
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to