Sessions: constantly asked to log in to access the Users Admin screen

Bug #1592236 reported by Ghada El-Zoghbi
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Dmitrii Metelkin
15.04
Fix Released
High
Unassigned
15.10
Fix Released
High
Unassigned
16.04
Fix Released
High
Unassigned
16.10
Fix Released
High
Dmitrii Metelkin

Bug Description

Mahara: 16.04.1
DB: Postgres
OS: Linux
Browser: Firefox

I think there may be something funny with sessions happening. When the session times out (it's currently set to 2 hours) and I'm on the /admin/users/search.php, I think it doesn't correctly update the session once I log in again.

This is what I think the steps are to reproduce it:
1. leave screen on /admin/users/search.php

2. have the session time out

3. try to do something on the screen (i.e. search for a user)

4. It will redirect to the login screen.

5. Login correctly

6. user search screen is displayed.

7. click on the Configure Site menu link

8. Click back on the Users menu link

9. It will ask you to log back in again.

It seems to only happen on the Users (and Group - I think) menu links.

Thanks.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Hi Ghada,

I haven't been able to replicate this issue locally using a clean install of 16.04.1. I did essentially what you described above:

1. Update my config.php with "cfg->session_timeout=10;" so that sessions will time out in 10 seconds, to make testing easier.
2. Clean install of Mahara
3. Log in to Mahara as an admin
4. Go to "Administration -> Users" (admin/users/search.php)
5. Wait for 15 seconds for the session to time out
6. Click the "Administration -> Users" search tab again
7. I get logged out due to my expired session, and I see the transient login form.
8. I log in at this form.
9. I again see the "Administration -> Users" screen
10. I click on "Administration -> Configure Site"
11. I click on "Administration -> Users" and "Administration -> Groups"

Expected error result: I expected that clicking on "Administration -> Users" or "Administration -> Groups" would take me back to the transient login screen again

Actual result: No matter which link I clicked on, I saw the correct screen. (So long as I was not waiting longer than my 10-second timeout to do the click.)

I also tried running the "auth_remove_old_session_files" Mahara cron task in between steps #10 and #11 above, to see if that was a necessary step for the error, but that didn't result in replicating the error either.

Changed in mahara:
status: New → Incomplete
Revision history for this message
Ghada El-Zoghbi (ghada-z) wrote :

Thanks Aaron.

I've just had it happen again. I checked my cookies and I end up with two 'mahara' cookies.

1. path: /admin/site
   Expires: at end of session
2. path: /
   Expires: at end of session

I think it's not clearing the cookies correctly.

So, if I log in from the main page, then the session expires while I'm in the /admin/site area, it's not clearing up the cookies correctly.

I hope that makes sense.

Cheers,
Ghada

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Ah, now that is interesting. If the cookies have different paths, then that would indeed explain the problem. In the browser, a cookie is uniquely identified by its name, domain, and path.

Mahara is *supposed* to set the session cookie's path to be only the path portion of $CFG->wwwroot, so that all Mahara pages will use the same session cookie. Somehow your site is failing to do that under certain circumstances, and instead sending out ones with "/admin/site", which the browser then interprets as an entirely separate cookie.

Hm, it gives me something more specific to look at, but I still can't replicate it.

Changed in mahara:
status: Incomplete → In Progress
status: In Progress → Triaged
Aaron Wells (u-aaronw)
Changed in mahara:
milestone: none → 16.04.2
Revision history for this message
Aaron Wells (u-aaronw) wrote :

This one's still a problem. The working hypothesis is that something loads up auth/session.php earlier than expected, when $CFG->wwwroot isn't filled in yet, and this causes a session cookie header to be sent out with the default value for the cookie's "path" component. The default is everything between the first and last "/" in the URL. So, if you're visiting an admin page, path "/admin/site/".

Thus a cookie with the name "mahara" and the path "/admin/site/" gets stored by the browser.

Then the user logs in. This causes their session ID to regenerate to a new one. This is handled by sending out a new cookie with the same, domain, and path. But this *doesn't* override the "/admin/site/" cookie, because it has a different path.

Now the user has two cookies named "browser" in their browser's cookie store. One at "/" and one at "/admin/site/". If they visit the page "/admin/site/index.php", their browser will send both cookies, with the "/admin/site/" one first, because it's more specific.

The problem is, PHP doesn't handle multiple session cookies with the same name well. It just reads the first one, and ignores the subsequent ones. The first one is "/admin/site/", with the old session ID. The old session ID is not recognized as valid, so the user is force-logged out.

The big question is, though, how exactly does a header with the wrong path get sent out? In theory, our code to delete duplicate session cookie headers from the response, should delete the ones with the bad path as well. And in theory, when we call the code in auth/session.php that names the cookie "mahara", that same chunk of code should compute the proper path for it based on the wwwroot, which seldom changes.

Revision history for this message
Aaron Wells (u-aaronw) wrote :

Okay, I think part of the problem here is that the function we use for determining the path component of $CFG->wwwroot (and hence the path component of the cookie's path) is buggy if your $CFG->wwwroot has no path component or trailing path (e.g.: "http://mahara.example.com" instead of "http://mahara.example.com/" or "http://example.com/mahara/")

In such a case, the function get_mahara_install_subdirectory() will return the whole domain name, (e.g. "mahara.example.com"). This then gets set as the cookie's path. Since it doesn't usually match the actual path component of the URL requested by the browser, the browser ignores it and instead applies the cookie to the "default path" for the requested URL.

So, with that in mind, I haven't been able to replicate this naturally, but I have been able to replicate it with a slight hack to force a bad path session header to be generated during the vulnerable period.

1. Set up your Mahara site so that in your config.php you have a $cfg->wwwroot with no path component or trailing slash (e.g.: $cfg->wwwroot = 'http://mahara';)

2. In htdocs/init.php, on line 201 (right after the call to "$SESSION = Session::singleton()"), add a line that reads:

$SESSION->set('foo', 'bar');

3. Log out of Mahara.

4. Go to http://mahara/admin/index.php

5. Because you're logged out, you'll see the transient login page. Log in here.

6. Now you're logged in, click around and everything should work.

7. Log out. You should now see the standard "logged out" homepage.

8. Log back in from the "Logged out" homepage.

9. Click on the Administration link.

Expected result: You should see the admin home page.

Actual result: You see the login screen again, as if you weren't logged out. If you examine your browser's cookie store now, you will see a mahara session cookie for the path "/" and another for the path "/admin/".

Changed in mahara:
assignee: nobody → Dmitrii Metelkin (dmitriim)
status: Triaged → In Progress
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

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

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/6648
Committed: https://git.mahara.org/mahara/mahara/commit/23318637f6fbd7c8fa0ba29d79e7acd88488f933
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 23318637f6fbd7c8fa0ba29d79e7acd88488f933
Author: Dmitrii Metelkin <email address hidden>
Date: Tue Jul 5 10:43:17 2016 +1000

Bug#1592236: initialise a session after wwwroot is set

behatnotneeded

Change-Id: Ia73346aa5a71952ee01d4955b864c7f2573d4a03

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

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

Revision history for this message
Mahara Bot (dev-mahara) wrote :

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

Revision history for this message
Mahara Bot (dev-mahara) wrote :

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

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/6652
Committed: https://git.mahara.org/mahara/mahara/commit/d36efa8e4f33f54086f097cb00717c34e70da696
Submitter: Ghada El-Zoghbi (<email address hidden>)
Branch: 16.04_STABLE

commit d36efa8e4f33f54086f097cb00717c34e70da696
Author: Dmitrii Metelkin <email address hidden>
Date: Tue Jul 5 10:43:17 2016 +1000

Bug#1592236: initialise a session after wwwroot is set

behatnotneeded

Change-Id: Ia73346aa5a71952ee01d4955b864c7f2573d4a03
(cherry picked from commit 23318637f6fbd7c8fa0ba29d79e7acd88488f933)

Revision history for this message
Mahara Bot (dev-mahara) wrote :

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

commit f41f5f7415b7a30667552aff9b295bce20f77fe0
Author: Dmitrii Metelkin <email address hidden>
Date: Tue Jul 5 10:43:17 2016 +1000

Bug#1592236: initialise a session after wwwroot is set

behatnotneeded

Change-Id: Ia73346aa5a71952ee01d4955b864c7f2573d4a03
(cherry picked from commit 23318637f6fbd7c8fa0ba29d79e7acd88488f933)

Revision history for this message
Mahara Bot (dev-mahara) wrote :

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

commit cf415bb0a01bebe8bc839a255c6d34eaca8e9857
Author: Dmitrii Metelkin <email address hidden>
Date: Tue Jul 5 10:43:17 2016 +1000

Bug#1592236: initialise a session after wwwroot is set

behatnotneeded

Change-Id: Ia73346aa5a71952ee01d4955b864c7f2573d4a03
(cherry picked from commit 23318637f6fbd7c8fa0ba29d79e7acd88488f933)

Robert Lyon (robertl-9)
Changed in mahara:
status: Fix Committed → Fix Released
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 16.10.0 → none
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.