mahara-contributors team mailing list archive

[Aaron Wells <1606101@xxxxxxxxxxxxxxxxxx>]

To restate the problem here:

1. The LDAP sync cron task can be configured to suspend users. It does
so using the function "suspend_user()" in user.php

2. When you suspend a user, there's a column, "usr.suspendingcusr",
which is meant to record the user ID of the user who performed the
suspension.

3. If you don't provide the suspending user's ID when you call
suspend_user(), it defaults to $USER->get('id').

4. That's what happens when you run the LDAP sync cron. And since it
runs via CLI, $USER->get('id') is "0". So "usr.suspendingcusr" is set to
0.

5. A lot of existing code throughout Mahara checks to see whether a user
is suspended, by doing "if ($usr->suspendingcusr)" or "if
(!empty($usr->suspendingcusr))".

6. The value of 0 passes both those checks, hence that existing code
treats the user as if they were *not* suspended.


Now, we could go through the code and change all those places that check whether a user is suspended, so that they use usr.suspendedctime instead. But I think it will be less bug-prone if we instead change what we're storing from 0 to a negative number. That will pass the boolean checks, while still giving us a way to see that someone was suspended by a cron task.

The one wrinkle there is that suspend_user(), in generating the
suspension notification message, runs display_name() on the suspending
user's ID. And display_name() throws an InvalidArgumentException if you
pass in the ID of a user who doesn't exist. But we can patch
suspend_user() to take care of that.

** Changed in: mahara
   Importance: Undecided => Medium

** Changed in: mahara
    Milestone: None => 16.10.0

** Changed in: mahara
       Status: New => In Progress

** Changed in: mahara
     Assignee: (unassigned) => Aaron Wells (u-aaronw)

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1606101

Title:
  LDAP user sync using root ID to suspend users

Status in Mahara:
  In Progress

Bug description:
  Mahara: 16.04.2
  DB: Postgres
  OS: Linux

  There is a bug with the LDAP sync when it is suspending users:
  htdocs/auth/ldap/cli/sync_users.php

  The user that is running the cron LDAP sync job is 'root' - has an ID
  of 0.

  Mahara updates the suspended user record with the userid that is doing
  the suspending (i.e. suspendedcusr = 0).

  When validating if a user record is suspended, we check if the
  suspending user id is empty. Because the suspending user ID is '0', it
  thinks that to the suspended user is not suspended - when they should
  be suspended.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1606101/+subscriptions