[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Changed in: mahara Milestone: 16.10.0 => None ** Changed in: mahara/16.10 Status: Fix Committed => Fix Released ** Changed in: mahara Status: Fix Committed => Fix Released -- 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: Fix Released Status in Mahara 15.04 series: Fix Released Status in Mahara 15.10 series: Fix Released Status in Mahara 16.04 series: Fix Released Status in Mahara 16.10 series: Fix Released 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Changed in: mahara/15.04 Status: Fix Committed => Fix Released ** Changed in: mahara/15.10 Status: Fix Committed => Fix Released ** Changed in: mahara/16.04 Status: Fix Committed => Fix Released -- 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: Fix Committed Status in Mahara 15.04 series: Fix Released Status in Mahara 15.10 series: Fix Released Status in Mahara 16.04 series: Fix Released Status in Mahara 16.10 series: Fix Committed 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Changed in: mahara/15.04 Status: Confirmed => Fix Committed ** Changed in: mahara/15.10 Status: Confirmed => Fix Committed ** Changed in: mahara/16.04 Status: Confirmed => Fix Committed -- 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: Fix Committed Status in Mahara 15.04 series: Fix Committed Status in Mahara 15.10 series: Fix Committed Status in Mahara 16.04 series: Fix Committed Status in Mahara 16.10 series: Fix Committed 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Changed in: mahara/16.10 Status: In Progress => Fix Committed ** Changed in: mahara/16.10 Assignee: Aaron Wells (u-aaronw) => Ghada El-Zoghbi (ghada-z) -- 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: Fix Committed Status in Mahara 15.04 series: Confirmed Status in Mahara 15.10 series: Confirmed Status in Mahara 16.04 series: Confirmed Status in Mahara 16.10 series: Fix Committed 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Changed in: mahara/16.04 Status: New => Confirmed ** Changed in: mahara/15.10 Status: New => Confirmed ** Changed in: mahara/15.04 Status: New => Confirmed ** Changed in: mahara/16.04 Importance: Undecided => Medium ** Changed in: mahara/15.10 Importance: Undecided => Medium ** Changed in: mahara/15.04 Importance: Undecided => Medium ** Changed in: mahara/16.04 Milestone: None => 16.04.3 ** Changed in: mahara/15.10 Milestone: None => 15.10.5 ** Changed in: mahara/15.04 Milestone: None => 15.04.9 -- 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 Status in Mahara 15.04 series: Confirmed Status in Mahara 15.10 series: Confirmed Status in Mahara 16.04 series: Confirmed Status in Mahara 16.10 series: 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
** Also affects: mahara/15.04 Importance: Undecided Status: New ** Also affects: mahara/15.10 Importance: Undecided Status: New ** Also affects: mahara/16.10 Importance: Medium Assignee: Aaron Wells (u-aaronw) Status: In Progress ** Also affects: mahara/16.04 Importance: Undecided Status: New -- 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 Status in Mahara 15.04 series: New Status in Mahara 15.10 series: New Status in Mahara 16.04 series: New Status in Mahara 16.10 series: 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
I'll submit a patch for option 1. -- 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
Abandoned patch 6750 after discussing it with Ghada, who has convinced me that putting an invalid user ID in that column creates too many potential bugs. So right now our options are: 1. Arbitrarily pick one of the site admin users, and use their ID as the suspendedcusr. 2. Locate every place that checks (bool) usr.suspendedcusr, and change it to check (bool) usr.suspendedctime instead. Option 1 is easier, but it has the nasty side effect that, well, we're arbitrarily picking one admin user and very rarely using them as the suspendedcusr. The name of that admin user is included in the message that gets sent to the suspended user, and so depending on how things are set up in that site, this could cause confusion. (Like, "Why did Tony suspend me?"). Option 2 would be cleaner. Or better yet, add a "usr.suspended" column, which is 0 or 1, and check that instead. That would prevent future devs from accidentally going back to relying on suspendedcusr. But the downside to 2, is that suspendedcusr shows up 27 different times throughout the code, and sometimes does double-duty as a boolean and as the ID of the suspending user, in ways that may be tricky to debug and test. So maybe, since the LDAP sync is the only way this situation comes up, it would be best to just stick with the low-effort Option 1? -- 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 ___ Mailing list: https://launchpad.net/~mahara-contributors Post to : mahara-contributors@lists.launchpad.net Unsubscribe : https://launchpad.net/~mahara-contributors More help : https://help.launchpad.net/ListHelp
[Mahara-contributors] [Bug 1606101] Re: LDAP user sync using root ID to suspend users
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
___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help : https://help.launchpad.net/ListHelp
