Session referer check should not be set if using SAML

[Jake Blatchford]

I'm using the SAML plugin for authentication and I've noticed that this change: https://reviews.mahara.org/#/c/5574/ introduces some new settings for PHP's session handling that cause issues with the SAML login process.

I believe only 16.04(rc1) is affected as that change is not present in 15.10.2.

The setting "session.referer_check" inteferes with the SAML login process and as a result the login process fails and an error is displayed to the user.

The particular line from the commit mentioned above is:

 - htdocs/auth/session.php
ini_set('session.referer_check', get_config('wwwroot'));

This option should not be set for most users if they are using SAML as an authentication method (in my case I am using a custom SAML auth plugin). During the login process SAML will redirect the user away from the wwwroot and when the user returns to Mahara the session data is cleared. This causes the "populate" function in the "LiveUser" class to attempt to create a new user using the default attributes (empty fields for username/firstname/lastname/etc). In addition the "usr" table does not have not have NOT NULL set on the username attribute, so an entry is created in the database with a null username causing various issues within Mahara.

I'd suggest adding a "session_referer_enabled" configuration option (just in config.php) that defaults to the referer check being enabled. This would allow it to easily be disabled by users who do not want it set without having to manually edit the htdocs/auth/session.php file. I'm not sure if there is a reason NOT_NULL is not set on the username field but maybe this should be changed as well?

[Aaron Wells (u-aaronw)]

Hi Jake,

Thanks for the bug report! You are correct, not only does that "session.referer_check" kill SAML, it also means that if you navigate to your Mahara site via a link (say, from an email), you get logged out.

We put that in there because the patch was based on the recommendations in the PHP manual's "securing sessions" page: http://php.net/manual/en/session.security.php

... but that page also points out that the setting is only helpful if you've turned on session.use_trans_id. And we have always had session.use_trans_id turned off, therefore we don't also need session.referer_check.

So, I will push a patch to get rid of that.

Cheers,
Aaron

[Mahara Bot (dev-mahara)]

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

[Aaron Wells (u-aaronw)]

To test:

1. Install Mahara
2. Log in to Mahara
3. Put a link to your Mahara into an HTML file somewhere outside your Mahara site's wwwroot (since my Mahara is at http://localhost/mahara/htdocs, I put a link to it in a file at http://localhost/test.html ). The link must *not* have the "rel=noreferrer" or "rel=nofollow" setting on it.
4. Click on the link, taking you to Mahara

Expected result: You're still logged in to Mahara
Actual result: You are logged out of Mahara

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6326
Committed: https://git.mahara.org/mahara/mahara/commit/91807920f4fb2981e1faa4978342d07674590d18
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 91807920f4fb2981e1faa4978342d07674590d18
Author: Aaron Wells <email address hidden>
Date: Tue Apr 12 15:46:28 2016 +1200

Remove session.referer_check (Bug 1566366)

This setting kills your Mahara session whenever you navigate
to Mahara from a link or redirect on another page. This totally
prevents SAML and other redirect-based auth methods from working,
makes it annoying to use links in email, and while it is mentioned
on the PHP manual's "Securing Sessions" page, it's only
recommended there if you also have "session.use_trans_id" enabled,
which we do not.

Change-Id: I8b3b14bae8043c5004cc8f36766f2db9422eac1c
behatnotneeded: Can't be tested by behat

[Mahara Bot (dev-mahara)]

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

[Mahara Bot (dev-mahara)]

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

[Mahara Bot (dev-mahara)]

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

commit 37dced259ed3f494180b451aef6110fc05897ba0
Author: Aaron Wells <email address hidden>
Date: Tue Apr 12 15:46:28 2016 +1200

Remove session.referer_check (Bug 1566366)

This setting kills your Mahara session whenever you navigate
to Mahara from a link or redirect on another page. This totally
prevents SAML and other redirect-based auth methods from working,
makes it annoying to use links in email, and while it is mentioned
on the PHP manual's "Securing Sessions" page, it's only
recommended there if you also have "session.use_trans_id" enabled,
which we do not.

Change-Id: I8b3b14bae8043c5004cc8f36766f2db9422eac1c
behatnotneeded: Can't be tested by behat
(cherry picked from commit 91807920f4fb2981e1faa4978342d07674590d18)

[Mahara Bot (dev-mahara)]

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

commit c9b8ff0208356676feb5bd0c65873c9f19a73681
Author: Aaron Wells <email address hidden>
Date: Tue Apr 12 15:46:28 2016 +1200

Remove session.referer_check (Bug 1566366)

This setting kills your Mahara session whenever you navigate
to Mahara from a link or redirect on another page. This totally
prevents SAML and other redirect-based auth methods from working,
makes it annoying to use links in email, and while it is mentioned
on the PHP manual's "Securing Sessions" page, it's only
recommended there if you also have "session.use_trans_id" enabled,
which we do not.

Change-Id: I8b3b14bae8043c5004cc8f36766f2db9422eac1c
behatnotneeded: Can't be tested by behat
(cherry picked from commit 91807920f4fb2981e1faa4978342d07674590d18)

[Mahara Bot (dev-mahara)]

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

[Mahara Bot (dev-mahara)]

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

commit bcdd15eadeda3442518bea9c5d822bc07541bcbf
Author: Aaron Wells <email address hidden>
Date: Tue Apr 12 15:46:28 2016 +1200

Remove session.referer_check (Bug 1566366)

This setting kills your Mahara session whenever you navigate
to Mahara from a link or redirect on another page. This totally
prevents SAML and other redirect-based auth methods from working,
makes it annoying to use links in email, and while it is mentioned
on the PHP manual's "Securing Sessions" page, it's only
recommended there if you also have "session.use_trans_id" enabled,
which we do not.

Change-Id: I8b3b14bae8043c5004cc8f36766f2db9422eac1c
behatnotneeded: Can't be tested by behat
(cherry picked from commit 91807920f4fb2981e1faa4978342d07674590d18)
(cherry picked from commit c9b8ff0208356676feb5bd0c65873c9f19a73681)

[Mahara Bot (dev-mahara)]

Patch for "1.10_STABLE" branch: https://reviews.mahara.org/6330

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6330
Committed: https://git.mahara.org/mahara/mahara/commit/902429569d07ea8f483d8f0a835c66a54b445f8a
Submitter: Robert Lyon (<email address hidden>)
Branch: 1.10_STABLE

commit 902429569d07ea8f483d8f0a835c66a54b445f8a
Author: Aaron Wells <email address hidden>
Date: Tue Apr 12 15:46:28 2016 +1200

Remove session.referer_check (Bug 1566366)

This setting kills your Mahara session whenever you navigate
to Mahara from a link or redirect on another page. This totally
prevents SAML and other redirect-based auth methods from working,
makes it annoying to use links in email, and while it is mentioned
on the PHP manual's "Securing Sessions" page, it's only
recommended there if you also have "session.use_trans_id" enabled,
which we do not.

Change-Id: I8b3b14bae8043c5004cc8f36766f2db9422eac1c
behatnotneeded: Can't be tested by behat
(cherry picked from commit 91807920f4fb2981e1faa4978342d07674590d18)
(cherry picked from commit c9b8ff0208356676feb5bd0c65873c9f19a73681)
(cherry picked from commit bcdd15eadeda3442518bea9c5d822bc07541bcbf)