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/