Users can login to suspended institutions via external auth under some circumstances

Bug #1580399 reported by Robert Lyon on 2016-05-11
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

The problem is this:

The code that checks if the user's authinstance is from a suspended institution, is in LiveUser->login(). This is the method used by the username/password login box.

But if you login with an auth method that doesn't use the login box, say SAML, XMLRPC, Shibboleth, you don't hit that check.

We need to move the check into the "ensure_user_account_is_active()" method in auth/lib.php which is already called at the start of LiveUser->authenticate() so we should update that to make sure it checks that their auth institution isn't suspended and (maybe) remove the redundant code from LiveUser->login()

See also for some more information about this issue
That bug report is public but I'll mark this as private as it mentions the attack vector

CVE References

Robert Lyon (robertl-9) on 2016-07-07
Changed in mahara:
status: Confirmed → In Progress
Aaron Wells (u-aaronw) wrote :

To test:

1. Install Moodle and Mahara. Enable Mnet on both.
2. Create an institution in Mahara with an XMLRPC auth instance, using Moodle and the "They SSO in" setting.
3. Set up Moodle so that your users can roam over to Mahara via MNet.
4. Set the Mahara institution's expiration date to the past, and/or suspend the Mahara institution.
5. Log in to Moodle.
6. Roam over to Mahara.

Expected result: When you try to roam over to Mahara, you can't log in, because your institution is expired.

Actual result: You roam to Mahara with no problem.

Submitter: Robert Lyon (<email address hidden>)
Branch: 15.10_STABLE

commit 792fb47bdc4dbbb058a2b5e6ac7dc9483cb4be34
Author: Robert Lyon <email address hidden>
Date: Fri Jul 8 09:05:53 2016 +1200

Bug 1580399: Stop users logging in to suspended/expired institutions

Moving the code from LiveUser->login() to
ensure_user_account_is_active() so that internal and external logins
can use the same code. This means the check now will fall after
LiveUser->authenticate() so a user's lastlogin values will be updated.
but that should be ok as the login was successful, it's just they
can't go any further as their institution is not active.


Change-Id: Ie78a60978d5936f78af5a962ca3efdcdee148b93
Signed-off-by: Robert Lyon <email address hidden>

Robert Lyon (robertl-9) on 2016-07-11
information type: Private Security → Public Security
Changed in mahara:
status: In Progress → Fix Committed
Robert Lyon (robertl-9) on 2016-10-21
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers