There is a bug with the way time-out is implemented. Time-out time is set as a particular clock time on the server but evaluated on the client machine. As a result if the client machine's clock is set forward in time by a larger amount than the login time-out they cannot log in. Detailed example below. (Assuming a 3 hour timeout, but issue is valid for any timeout length.)

  1. User attempts to log in (11:59 pm GMT according to user's clock)
  2. Server validates login (11:59 am GMT according to server's clock)
  3. Server sends user cookie for valid login that expires at 2:59 pm GMT
  4. User's computer notes that 2:59 pm is before 11:59 pm. Thus the cookie is already expired. Therefore cookie gets immediatly discarded
  5. User's computer attempts to visit logged in page without cookie
  6. Server says hey you are not logged in and sends back error message
  7. User curses your broken website and never comes back*

* Or if you are lucky tells you that it is impossible to login. Which you cannot replicate.

A few extra notes about this issue.

  • There is a version of Firefox that does not exhibt this problem. It attempts to auto correct the problem. However, that is no longer the case so I'm guessing it is "fixed" in current versions of Firefox.
  • I have validated this issue in IE 9, Opera 10.61, Firefox 3.6.13, Safari 5.0.2 (for Windows), Chrome 12.0.712.0 and Lynx 2.8.6rel.1 (for Windows)
  • User timezone does not matter.
    • The server cookie is explictly sent in GMT
    • The user's clock must be incorrect within their timezone as a result.
  • A surprising number of user's clocks are wrong
  • Users do not like being told that their clock is wrong
  • User's don't like not being able to log in when it is required of them
  • I have strayed from useful bug related information and will stop now

Comments

coltrane’s picture

It's not clear to me that this is an issue in the Bakery code.

antgiant’s picture

Perhaps this is a clearer way to describe the problem.
The Bakery cookie is set in two places

993:  setcookie('CHOCOLATECHIP', bakery_mix(serialize($cookie), 1), $_SERVER['REQUEST_TIME'] + variable_get('bakery_freshness', '3600'), '/', variable_get('bakery_domain', ''));</
1038: setcookie('OATMEAL', bakery_mix(serialize($cookie), 1), $_SERVER['REQUEST_TIME'] + variable_get('bakery_freshness', '3600'), '/', variable_get('bakery_domain', ''));

If both cases you'll notice that the third parameter is $_SERVER['REQUEST_TIME'] + variable_get('bakery_freshness', '3600').

However, the code that verifies the cookie is stil valid looks like this.

968: if ($signature == $cookie['signature'] && $cookie['timestamp'] + variable_get('bakery_freshness', '3600') >= $_SERVER['REQUEST_TIME']) {

Due to the joys of working with cookies this means that every Bakery cookie effectivly has 2 expiration dates.

  1. When $cookie['timestamp'] + variable_get('bakery_freshness', '3600') >= $_SERVER['REQUEST_TIME']
  2. When the expiration date set on the cookie is in the past according to the client computer.

That second option is the one that is causing me trouble. As far as I can tell there are two ways to solve this problem.

  1. Use some javascript to collect the current time on the client's computer, and use that timestamp for setting the cookie expiration.
  2. Set the expiration date really far in the future and take advantage of the fact that the first method of expiration will properly expire it.
greggles’s picture

Of your two solutions I think #2 is better. Relying on javascript for a session related feature seems like a poor choice.

Can you post a patch of #2?

antgiant’s picture

Status: Active » Needs review
StatusFileSize
new1.77 KB

Patch changes cookie expiration on client computer to 10 years beyond default expiration. That should resolve all but the most extreme of incorrect client clock problems.

greggles’s picture

Status: Needs review » Needs work

This needs comments as to why we are doing it.

1. By the setcookie comments explaining 10 years of time
2. By whatever code will get rid of these cookies in the timeframe of less than 10 years time

antgiant’s picture

For some reason I don't quite understand my copy of Git will only create multiple patches for me. I'm probably doing something wrong. Hope that is not to much of a problem.

First patch is unchanged from above. Second patch adds comments requested by greggles.

antgiant’s picture

Status: Needs work » Needs review

Forgot to mark it as needs review. Sorry.

coltrane’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.48 KB

Reroll with mods to comments. RTBC.

coltrane’s picture

Status: Reviewed & tested by the community » Needs review

Well, I want to think on this a little more and run some tests.

purencool’s picture

Version: 6.x-2.0-alpha2 » 3.0.0-alpha1
Status: Needs review » Active

Test to see if this still is happening in the latest version.

avpaderno’s picture

Version: 3.0.0-alpha1 » 3.x-dev
Status: Active » Needs work
Issue tags: +Needs reroll, +Needs merge request