Mahara not respecting session lifetime setting from admin config page

[Aaron Wells]

It seems that after the last round of session fixing bugs, Mahara no longer respects the session lifetime setting that the admin can set on the site configuration page.

This setting is stored in the database config table as "session_timeout". It's then retrieved from the database during session setup, and loaded into the "session.gc_maxlifetime" ini value.

The problem is, we are now initiating the session *before* we launch the database connection. So when we are setting session.gc_maxlifetime, session_timeout isn't available, and instead we use the default value of 1440 seconds = 24 minutes.

The quick workaround is to add your session_timeout setting to your config.php:

$cfg->session_timeout = 14400; // session timeout of 4 hours

[Aaron Wells (u-aaronw)]

This bug is probably more noticeable if you're using the memcache session handler rather than the default (filesystem) session handler. I think the memcache session handler uses session.gc_maxlifetime as the hard expiration date for sessions; whereas in the default filesystem session handler they may stick around until the garbage handler runs.

[Aaron Wells (u-aaronw)]

Okay, on further inspection into the PHP source code, it's apparent this bug is even less of a problem (for non-memcache users) than I thought.

In the default "files" session handler in PHP, the garbage collector actually doesn't do *anything* if your session directory depth is more than 0. It just short-circuits out (see https://github.com/php/php-src/blob/PHP-5.5.9/ext/session/mod_files.c#L445 ), and relies on an external cron job to clean out old session files. The code for this is virtually unchanged from PHP 5.3 to PHP 7.

The session directory depth referred to here, is the optional first part of the "session.save_path" ini setting, which Mahara sets to 3. So Mahara cleans out its old session files using a cron script, auth_remove_old_session_files() in auth/lib.php, which is set to run once a day. Weirdly, although that function is operating in lieue of the core PHP session cleaning function, it is hard-coded to add 2 days to the session lifetime before it clears out old files!

[Aaron Wells (u-aaronw)]

Did a quick check to see whether the 2 days added on was copied from the core session cleanup file. It seems that PHP itself doesn't ship with a shell script for cleaning up session files. Debian does (it's the cron at /etc/cron.d/php5), and it doesn't add 2 days to that time limit. So I'm not sure where that came from.

[Aaron Wells (u-aaronw)]

Okay, after a thorough investigation of how we actually are timing out sessions, I've found a few issues and inconsistencies in it. Since this bug is only "Medium" (for non-memcache sites), I'm going to fix the core bug here with a minor patch that only changes init.php to move session initiation down below database initiation.

Then, I'll spin off the other issues into a separate bug, Bug 1590293.

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/6565

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6565
Committed: https://git.mahara.org/mahara/mahara/commit/12cb73cf14e7cd0fb5e4b03615e0dfb501bf86c3
Submitter: Son Nguyen (<email address hidden>)
Branch: master

commit 12cb73cf14e7cd0fb5e4b03615e0dfb501bf86c3
Author: Aaron Wells <email address hidden>
Date: Wed Jun 8 19:09:58 2016 +1200

Bug 1588613: Later session start so we can use DB config table

The session init code relies on $CFG->session_timeout, which is
normally defined in the config table. So, we need to start the
session after opening the database connection.

(In the event that there's an earlier session start, for instance
due to an error message, this will cause the session for that
page load to disregard any database config values. But that's not
a show-stopper, and there's no easy way to fix it.)

Change-Id: Iffbeebc8e92929970a558ff0fbc726719bb92741
behatnotneeded: Covered by existing tests

[Mahara Bot (dev-mahara)]

Patch for "16.04_STABLE" branch: https://reviews.mahara.org/6575

[Mahara Bot (dev-mahara)]

Patch for "15.10_STABLE" branch: https://reviews.mahara.org/6576

[Mahara Bot (dev-mahara)]

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/6577

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6575
Committed: https://git.mahara.org/mahara/mahara/commit/7f697c51b48d3177ad239a18d1826235a27eda60
Submitter: Robert Lyon (<email address hidden>)
Branch: 16.04_STABLE

commit 7f697c51b48d3177ad239a18d1826235a27eda60
Author: Aaron Wells <email address hidden>
Date: Wed Jun 8 19:09:58 2016 +1200

Bug 1588613: Later session start so we can use DB config table

The session init code relies on $CFG->session_timeout, which is
normally defined in the config table. So, we need to start the
session after opening the database connection.

(In the event that there's an earlier session start, for instance
due to an error message, this will cause the session for that
page load to disregard any database config values. But that's not
a show-stopper, and there's no easy way to fix it.)

Change-Id: Iffbeebc8e92929970a558ff0fbc726719bb92741
behatnotneeded: Covered by existing tests
(cherry picked from commit 12cb73cf14e7cd0fb5e4b03615e0dfb501bf86c3)

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6577
Committed: https://git.mahara.org/mahara/mahara/commit/e5e7b38cca4f4c3e35a47b14c0b93cf93d370672
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.04_STABLE

commit e5e7b38cca4f4c3e35a47b14c0b93cf93d370672
Author: Aaron Wells <email address hidden>
Date: Wed Jun 8 19:09:58 2016 +1200

Bug 1588613: Later session start so we can use DB config table

The session init code relies on $CFG->session_timeout, which is
normally defined in the config table. So, we need to start the
session after opening the database connection.

(In the event that there's an earlier session start, for instance
due to an error message, this will cause the session for that
page load to disregard any database config values. But that's not
a show-stopper, and there's no easy way to fix it.)

Change-Id: Iffbeebc8e92929970a558ff0fbc726719bb92741
behatnotneeded: Covered by existing tests
(cherry picked from commit 12cb73cf14e7cd0fb5e4b03615e0dfb501bf86c3)

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6576
Committed: https://git.mahara.org/mahara/mahara/commit/3247e26c153e998b63420bc9b101a4553220a1bc
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit 3247e26c153e998b63420bc9b101a4553220a1bc
Author: Aaron Wells <email address hidden>
Date: Wed Jun 8 19:09:58 2016 +1200

Bug 1588613: Later session start so we can use DB config table

The session init code relies on $CFG->session_timeout, which is
normally defined in the config table. So, we need to start the
session after opening the database connection.

(In the event that there's an earlier session start, for instance
due to an error message, this will cause the session for that
page load to disregard any database config values. But that's not
a show-stopper, and there's no easy way to fix it.)

Change-Id: Iffbeebc8e92929970a558ff0fbc726719bb92741
behatnotneeded: Covered by existing tests
(cherry picked from commit 12cb73cf14e7cd0fb5e4b03615e0dfb501bf86c3)