Heh, consistency on the permission stuff would be nice, wouldn't it? :)
It's been such a long time since I looked at the permission stuff --
your email made me go a look at it, and there is some code rot in there
indeed. I think the intent was (and the implementation should be
something like) to have have_* functions return a boolean so the calling
code can decide what to (presumably non-fatal) and have the check_*
functions display the error and stop execution. I believe the others
were intended to be private functions, but again, that was a long time
ago now. I'll be happy to review the permission stuff to try to make it
more logical.
I had intended the Admin bit to override all others, so that Admins
would be able to do anything regardless of whether they had the specific
permissions.
I think I made a commit recently that reduces the significance of the
User group. I'll poke at that as well and take a look at the admin
error template.
Ulf Erikson wrote:
> Hoi Ben,
>
> I have gather a few thoughts and comments from the last week..
>
> Permission is checked in several different ways:
> $perm->have_*, $perm->is_*, $perm->in_*, $perm->check_*
> The check_* functions often, but not always, show an error page if the
> permission is not granted. The other functions return a boolean. Will
> you please take a look at this and make it more consistent to make
> things easier to understand?
>
> The permission bits checked for are not always the correct ones:
> Manager, Administrator, Users
> Since the "Admin" bit automatically grants permission to everything are
> those functions still available. Please check again what requirements
> you had intended at those points.
>
> The User group is somewhat "magic". Everyone is included be default, and
> it never shows up in the administrator tools. Could this (that it
> doesn't show up) please be changed? It "breaks" the group editor
> somewhat that the User group is missing, and since a Developer has all
> permissions a User has is there no need to be member of both..
>
> From admin/user.php:
> // Get user's groups (without dropping the user group)
> $user_groups = $db->getCol(sprintf($QUERY['admin-user-groups'],
> $userid));
> but 'admin-user-group' DOES drop the user group. It shouldn't. (i think
> things went wrong when not everyone is a "User", or maybe even before)
>
> There is no error.html in the templates/admin/ directory. This is needed
> when/if something goes wrong with the database queries (which it at
> least does when i play with trial-and-error). Could this please be added?
>
|