Notes |
|
|
thanks for the correction. |
|
|
(0003765)
|
karstenmtr
|
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.
|
|
|
(0003767)
|
stevez
|
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. |
|
|
|
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.
|
|
|
(0003769)
|
stevez
|
2016-04-15 13:29
|
|
It's always nice when another pair of eyes verifies! Thanks karstenmtr |
|
|
|
|
|
|
|