| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
|---|---|---|---|---|---|---|---|---|---|
| 0001683 | v3.4 Release (Current) | Core Infrastructure | public | 2016-02-02 21:21 | 2019-01-03 12:53 | ||||
| Reporter | stevez | ||||||||
| Assigned To | caseydk | ||||||||
| Priority | high | Severity | major | Reproducibility | always | ||||
| Status | closed | Resolution | fixed | ||||||
| Platform | all | OS | all | OS Version | all | ||||
| Product Version | |||||||||
| Target Version | Fixed in Version | ||||||||
| Summary | 0001683: Administrators cannot set user preferences / only user can set preferences | ||||||||
| Description | http://support.web2project.net/questions/1951/access-denied-in-edit-user-preferences.html#comment-2621 | ||||||||
| Steps To Reproduce | login as any admin user / attempt to edit preferences for any other user ... (Access Denied) | ||||||||
| Additional Information | Module:\modules\system\do_preference_aed.php Line17: if ((!($AppUI->user_id == $pref_user) && !canEdit('admin')) && $pref_user) { $AppUI->redirect(ACCESS_DENIED); } Needs to change to: if ($AppUI->user_id != $pref_user) { if (!(canEdit('users'))) { $AppUI->redirect(ACCESS_DENIED); }; } | ||||||||
| Tags | No tags attached. | ||||||||
| Attached Files |
| ||||||||
 Relationships |
|
 Relationships |
| Notes |
|
|
Themoulos (reporter) 2016-02-07 12:16 |
thanks for the correction. |
|
karstenmtr (reporter) 2016-04-14 22:41 Last edited: 2016-04-15 13:17 |
The mentioned fix might work, but my guess is, that this fix doesn't represent the intended behavior and do not address the underlying changes in web2project, causing this bug. In my opinion the correct fix should be if ((!($AppUI->user_id == $pref_user) && !canEdit('users')) && $pref_user) { $AppUI->redirect(ACCESS_DENIED); } As you can see, I just changed the parameter of the canEdit function. This is because I think, there is no admin-module anymore. The permission check inside canEdit is done against the gacl_permissons table, which doesn't contain an module called admin. But there is one named users. And I think this is the right module to check the edit users permission against. Edit: I'd like to revoke my previous statement. It seems I was in a hurry and didn't catch the essence of the proposed fix. Thinking one more time about it, the fix including the rewritten statement is an improvement. The nested canEdit call should be an improvement, because the permission check against the database is only executed, when required. |
|
stevez (reporter) 2016-04-15 13:21 |
Decomposing the original "if" statement as follows yields one parsing problem. ( ( !($AppUI->user_id == $pref_user) && !canEdit('users') ) && $pref_user ) The evaluative order leaves $pref_user as a boolean which is not explicit and therefore possibly a source of problems later. All I was suggesting in my order was that the explicit test: ($AppUI->user_id != $pref_user) This form of the test may be more reliable (both semantically and logically). Your mileage may vary but my original fix has been working in production for a couple months now. |
|
karstenmtr (reporter) 2016-04-15 13:26 Last edited: 2016-04-15 13:26 |
Your completely right, stevez. I missed this today in the morning when I wrote my note, but I just edited my statement a few minutes ago as I got the point in your proposed fix. Sorry for my mistake. |
|
stevez (reporter) 2016-04-15 13:29 |
It's always nice when another pair of eyes verifies! Thanks karstenmtr |
|
caseydk (administrator) 2016-12-31 00:03 |
Nice catch! You are correct that the short fix was replacing 'admin' with 'user' but I made another change to the canEdit to make sure the user has the ability to edit that specific user. For example, where an Admin can manage their users but not other Admins. Resolved: https://github.com/web2project/web2project/commit/4ce991516793d5691ee6c5e37541c2bb1e121b17 |
|
caseydk (administrator) 2019-01-03 12:53 |
In the 31 Dec 2018 release: http://docs.web2project.net/release-notes/3.4.html |
| Notes |
| Issue History |
|||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-02-02 21:21 | stevez | New Issue | |
| 2016-02-07 12:16 | Themoulos | Note Added: 0003764 | |
| 2016-04-14 22:41 | karstenmtr | Note Added: 0003765 | |
| 2016-04-15 13:16 | karstenmtr | Note Edited: 0003765 | View Revisions |
| 2016-04-15 13:17 | karstenmtr | Note Edited: 0003765 | View Revisions |
| 2016-04-15 13:21 | stevez | Note Added: 0003767 | |
| 2016-04-15 13:26 | karstenmtr | Note Added: 0003768 | |
| 2016-04-15 13:26 | karstenmtr | Note Edited: 0003768 | View Revisions |
| 2016-04-15 13:29 | stevez | Note Added: 0003769 | |
| 2016-12-26 23:34 | caseydk | Project | v3.3 Release => v3.4 Release (Current) |
| 2016-12-29 23:49 | caseydk | Assigned To | => caseydk |
| 2016-12-29 23:49 | caseydk | Status | new => assigned |
| 2016-12-31 00:03 | caseydk | Status | assigned => resolved |
| 2016-12-31 00:03 | caseydk | Resolution | open => fixed |
| 2016-12-31 00:03 | caseydk | Note Added: 0003865 | |
| 2019-01-03 12:53 | caseydk | Note Added: 0003951 | |
| 2019-01-03 12:53 | caseydk | Status | resolved => closed |
| Issue History |