When a user is expired due to inactivity, they can still log in

Bug #890929 reported by Hugh Davenport
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Richard Mansfield

Bug Description

In the cronjob auth_handle_account_expiries, users that haven't logged in since the config option defaultaccountinactiveexpire get deactivated. This sets the active flag in the usr table.

One would think that being inactive would mean that the user could no longer login, but they still can. Especially because the email sent out before you become inactive is as follows:

"Dear Admin User (admin),

Your account on Mahara will become inactive within 7 days.

Once inactive, you will not be able to log in until an administrator re-enables your account.

You can prevent your account from becoming inactive by logging in.

Regards, Mahara Site Administrator

Please do not reply to this message.
"

Tags: expiry
Changed in mahara:
status: New → Triaged
importance: Undecided → Medium
milestone: none → 1.5.0
Revision history for this message
Melissa Draper (melissa) wrote :

Hugh,

I can't seem to reproduce this. Was your login before or after the 7 days specified?

Thanks,
Melissa

Revision history for this message
Hugh Davenport (hugh-davenport) wrote :

I think it had something funny with the cronjob running it needed time to rethink it

I would say it was after the 7 days, I actually set the defaultaccountinactiveexpire setting to something very low (so email got sent but wasn't actually an extra 7 days iirc). But I had to force the cronjob to run twice, so that it set the active flag to 0 in usr table.

Though we should probably wait the 7 days really, but yeh... I didn't want to wait that long :D

UPDATE cron SET nextrun=NULL WHERE callfunction='auth_handle_account_expiries'

Revision history for this message
François Marier (fmarier) wrote :

I think this code is wrong in auth/lib.php:login_submit():

    // Check if the user's account has become inactive
    $inactivetime = get_config('defaultaccountinactiveexpire');
    if ($inactivetime && $oldlastlogin > 0
        && $oldlastlogin + $inactivetime < time()) {
        $USER->logout();
        die_info(get_string('accountinactive'));
    }

Because $oldlastlogin is set to 0 at the top of the function and never changed. So the code within the "if" statement will never execute.

tags: added: expiry
Revision history for this message
François Marier (fmarier) wrote :

One thing to note is that we can't just fix the above code because then there would be no way for an admin to "unexpire" an inactive user.

We need to fix that UI omission as well.

Changed in mahara:
assignee: nobody → Richard Mansfield (richard-mansfield)
Changed in mahara:
status: Triaged → In Progress
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/958
Committed: http://gitorious.org/mahara/mahara/commit/abda43b5dbbc9a4051f3a6f48606f62036082289
Submitter: Francois Marier (<email address hidden>)
Branch: master

commit abda43b5dbbc9a4051f3a6f48606f62036082289
Author: Richard Mansfield <email address hidden>
Date: Thu Dec 22 15:03:05 2011 +1300

    Remove unused inactive check from login code (bug #890929)

    This code was never run because the $oldlastlogin variable was
    hardcoded to 0. The code can be removed altogether, and instead rely
    on cron to set expiry dates on inactive users.

    Change-Id: I9883a7720960f2337edf060197346cd17f0193c5
    Signed-off-by: Richard Mansfield <email address hidden>

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

Reviewed: https://reviews.mahara.org/959
Committed: http://gitorious.org/mahara/mahara/commit/81f262545f937074987581e5606cbe6aa8013c5c
Submitter: Francois Marier (<email address hidden>)
Branch: master

commit 81f262545f937074987581e5606cbe6aa8013c5c
Author: Richard Mansfield <email address hidden>
Date: Thu Dec 22 10:23:41 2011 +1300

    Expire users when they've been inactive for too long (bug #890929)

    The "Default account inactivity time" setting allows the admin to
    specify a time period after which users who have not used the site
    will be unable to login, but this is not currently enforced.

    This change modifies the inactivity cron job to set the expiry date to
    the current date for any user who has been inactive for longer than
    the 'defaultaccountinactiveexpire' period. It also now considers the
    lastaccess and ctime fields as well as the lastlogin field.

    This allows the admin to reactivate inactive users by resetting their
    expiry dates in account settings.

    The active column on the user table is currently only used to decide
    whether users should be displayed in search results, and users are set
    to inactive whenever they are deleted, suspended, or expire.

    Change-Id: Ieaf7a0b36865af726fc2526895146373efbb2741
    Signed-off-by: Richard Mansfield <email address hidden>

Changed in mahara:
status: In Progress → Fix Committed
Melissa Draper (melissa)
Changed in mahara:
status: Fix Committed → Fix Released
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.